Fix: prevent apply existing orphan resource (#4399)

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
This commit is contained in:
Somefive
2022-07-18 19:23:41 +08:00
committed by GitHub
parent b24e7523d8
commit e747eae779
7 changed files with 69 additions and 6 deletions

View File

@@ -1238,6 +1238,7 @@ var _ = Describe("Test Application Controller", func() {
"trait.oam.dev/type": "AuxiliaryWorkload",
"app.oam.dev/component": compName,
"app.oam.dev/name": app.Name,
"app.oam.dev/namespace": app.Namespace,
"trait.oam.dev/resource": "gameconfig",
"app.oam.dev/appRevision": app.Name + "-v1",
},
@@ -1260,6 +1261,7 @@ var _ = Describe("Test Application Controller", func() {
"trait.oam.dev/resource": "ingress",
"app.oam.dev/component": compName,
"app.oam.dev/name": app.Name,
"app.oam.dev/namespace": app.Namespace,
"app.oam.dev/appRevision": app.Name + "-v1",
},
},
@@ -1284,6 +1286,7 @@ var _ = Describe("Test Application Controller", func() {
"trait.oam.dev/resource": "service",
"app.oam.dev/component": compName,
"app.oam.dev/name": app.Name,
"app.oam.dev/namespace": app.Namespace,
"app.oam.dev/appRevision": app.Name + "-v1",
},
},

View File

@@ -362,6 +362,12 @@ func (h *AppHandler) ApplyPolicies(ctx context.Context, af *appfile.Appfile) err
return errors.Wrapf(err, "failed to render policy manifests")
}
if len(policyManifests) > 0 {
for _, policyManifest := range policyManifests {
util.AddLabels(policyManifest, map[string]string{
oam.LabelAppName: h.app.GetName(),
oam.LabelAppNamespace: h.app.GetNamespace(),
})
}
if err = h.Dispatch(ctx, "", common.PolicyResourceCreator, policyManifests...); err != nil {
return errors.Wrapf(err, "failed to dispatch policy manifests")
}

View File

@@ -37,6 +37,9 @@ const (
EnableSuspendOnFailure featuregate.Feature = "EnableSuspendOnFailure"
// LegacyComponentRevision if enabled, create component revision even no rollout trait attached
LegacyComponentRevision featuregate.Feature = "LegacyComponentRevision"
// LegacyResourceOwnerValidation if enabled, the resource dispatch will allow existing resource not to have owner
// application and the current application will take over it
LegacyResourceOwnerValidation featuregate.Feature = "LegacyResourceOwnerValidation"
// Edge Features
@@ -51,6 +54,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
LegacyResourceTrackerGC: {Default: false, PreRelease: featuregate.Beta},
EnableSuspendOnFailure: {Default: false, PreRelease: featuregate.Alpha},
LegacyComponentRevision: {Default: false, PreRelease: featuregate.Alpha},
LegacyResourceOwnerValidation: {Default: false, PreRelease: featuregate.Alpha},
AuthenticateApplication: {Default: false, PreRelease: featuregate.Alpha},
}

View File

@@ -28,11 +28,13 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
"github.com/oam-dev/kubevela/pkg/controller/utils"
"github.com/oam-dev/kubevela/pkg/features"
"github.com/oam-dev/kubevela/pkg/oam"
"github.com/oam-dev/kubevela/pkg/oam/util"
)
@@ -297,8 +299,11 @@ func MustBeControlledByApp(app *v1beta1.Application) ApplyOption {
return nil
}
appKey, controlledBy := GetAppKey(app), GetControlledBy(existing)
if controlledBy == "" && !utilfeature.DefaultMutableFeatureGate.Enabled(features.LegacyResourceOwnerValidation) {
return fmt.Errorf("%s %s/%s exists but not managed by any application now", existing.GetObjectKind().GroupVersionKind().Kind, existing.GetNamespace(), existing.GetName())
}
if controlledBy != "" && controlledBy != appKey {
return fmt.Errorf("existing object is managed by other application %s", controlledBy)
return fmt.Errorf("existing object %s %s/%s is managed by other application %s", existing.GetObjectKind().GroupVersionKind().Kind, existing.GetNamespace(), existing.GetName(), controlledBy)
}
return nil
}

View File

@@ -18,9 +18,14 @@ package apply
import (
"context"
"strings"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
@@ -28,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
"github.com/oam-dev/kubevela/pkg/oam"
oamutil "github.com/oam-dev/kubevela/pkg/oam/util"
)
@@ -128,6 +134,38 @@ var _ = Describe("Test apply", func() {
Expect(len(resultDeploy.Spec.Template.Spec.Volumes)).Should(Equal(1))
})
It("Test apply resource already exists", func() {
ctx := context.Background()
app := &v1beta1.Application{ObjectMeta: metav1.ObjectMeta{Name: "test-app", Namespace: "default"}}
By("Test create resource already exists but has no application owner")
cm1 := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-resource-exists", Namespace: "default"}}
Expect(rawClient.Create(ctx, cm1)).Should(Succeed())
Expect(rawClient.Get(ctx, client.ObjectKeyFromObject(cm1), &corev1.ConfigMap{})).Should(Succeed())
obj1, err := runtime.DefaultUnstructuredConverter.ToUnstructured(cm1)
Expect(err).Should(Succeed())
u1 := &unstructured.Unstructured{Object: obj1}
u1.SetGroupVersionKind(schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"})
Expect(k8sApplicator.Apply(ctx, u1, MustBeControlledByApp(app))).Should(Satisfy(func(err error) bool {
return err != nil && strings.Contains(err.Error(), "exists but not managed by any application now")
}))
Expect(rawClient.Delete(ctx, cm1)).Should(Succeed())
By("Test create resource already exists but owned by other application")
cm2 := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-resource-exists-owned-by-others", Namespace: "default"}}
oamutil.AddLabels(cm2, map[string]string{
oam.LabelAppName: "other",
oam.LabelAppNamespace: "default",
})
Expect(rawClient.Create(ctx, cm2)).Should(Succeed())
Expect(rawClient.Get(ctx, client.ObjectKeyFromObject(cm2), &corev1.ConfigMap{})).Should(Succeed())
obj2, err := runtime.DefaultUnstructuredConverter.ToUnstructured(cm2)
u2 := &unstructured.Unstructured{Object: obj2}
u2.SetGroupVersionKind(schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"})
Expect(err).Should(Succeed())
Expect(k8sApplicator.Apply(ctx, u2, MustBeControlledByApp(app))).Should(Satisfy(func(err error) bool {
return err != nil && strings.Contains(err.Error(), "is managed by other application")
}))
Expect(rawClient.Delete(ctx, cm2)).Should(Succeed())
})
})
})

View File

@@ -381,19 +381,19 @@ func TestMustBeControlledByApp(t *testing.T) {
},
"old app has no label": {
existing: &appsv1.Deployment{},
hasError: false,
hasError: true,
},
"old app has no app label": {
existing: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{},
}},
hasError: false,
hasError: true,
},
"old app has no app ns label": {
existing: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{oam.LabelAppName: "app"},
}},
hasError: false,
hasError: true,
},
"old app has correct label": {
existing: &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{

View File

@@ -107,6 +107,13 @@ var _ = Describe("Test multicluster standalone scenario", func() {
It("Test standalone app with publish version", func() {
By("Apply resources")
nsLocal := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace + "-local"}}
Expect(k8sClient.Create(hubCtx, nsLocal)).Should(Succeed())
defer func() {
_ = k8sClient.Delete(hubCtx, nsLocal)
}()
deploy := readFile("deployment.yaml")
Expect(k8sClient.Create(hubCtx, deploy)).Should(Succeed())
workflow := readFile("workflow-suspend.yaml")
@@ -152,7 +159,7 @@ var _ = Describe("Test multicluster standalone scenario", func() {
// update application without updating publishVersion
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(hubCtx, appKey, _app)).Should(Succeed())
_app.Spec.Policies[0].Properties = &runtime.RawExtension{Raw: []byte(`{"clusters":["local"]}`)}
_app.Spec.Policies[0].Properties = &runtime.RawExtension{Raw: []byte(fmt.Sprintf(`{"clusters":["local"],"namespace":"%s"}`, nsLocal.Name))}
g.Expect(k8sClient.Update(hubCtx, _app)).Should(Succeed())
}, 10*time.Second).Should(Succeed())
@@ -181,7 +188,7 @@ var _ = Describe("Test multicluster standalone scenario", func() {
deploys := &v1.DeploymentList{}
g.Expect(k8sClient.List(workerCtx, deploys, client.InNamespace(namespace))).Should(Succeed())
g.Expect(len(deploys.Items)).Should(Equal(0))
g.Expect(k8sClient.List(hubCtx, deploys, client.InNamespace(namespace))).Should(Succeed())
g.Expect(k8sClient.List(hubCtx, deploys, client.InNamespace(nsLocal.Name))).Should(Succeed())
g.Expect(len(deploys.Items)).Should(Equal(1))
g.Expect(deploys.Items[0].Spec.Replicas).Should(Equal(pointer.Int32(3)))
}, 30*time.Second).Should(Succeed())