fix app finalizer bug (#1962)

Signed-off-by: roy wang <seiwy2010@gmail.com>
This commit is contained in:
Yue Wang
2021-07-26 14:20:22 +08:00
committed by GitHub
parent 5d6ce83174
commit c0144865d2
2 changed files with 76 additions and 25 deletions
@@ -38,7 +38,6 @@ import (
velatypes "github.com/oam-dev/kubevela/apis/types"
"github.com/oam-dev/kubevela/pkg/appfile"
core "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev"
"github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2/application/dispatch"
"github.com/oam-dev/kubevela/pkg/controller/utils"
"github.com/oam-dev/kubevela/pkg/cue/packages"
"github.com/oam-dev/kubevela/pkg/oam"
@@ -59,9 +58,9 @@ const (
legacyResourceTrackerFinalizer = "resourceTracker.finalizer.core.oam.dev"
// resourceTrackerFinalizer is to delete the resource tracker of the latest app revision.
resourceTrackerFinalizer = "app.oam.dev/resource-tracker-finalizer"
// onlyRevisionFinalizer is to delete all resource trackers of app revisions which may be used
// legacyOnlyRevisionFinalizer is to delete all resource trackers of app revisions which may be used
// out of the domain of app controller, e.g., AppRollout controller.
onlyRevisionFinalizer = "app.oam.dev/only-revision-finalizer"
legacyOnlyRevisionFinalizer = "app.oam.dev/only-revision-finalizer"
)
// Reconciler reconciles a Application object
@@ -253,14 +252,6 @@ func (r *Reconciler) handleFinalizers(ctx context.Context, app *v1beta1.Applicat
klog.InfoS("Register new finalizer for application", "application", klog.KObj(app), "finalizer", resourceTrackerFinalizer)
return true, errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)
}
if appWillRollout(app) {
klog.InfoS("Found an application which will be released by rollout", "application", klog.KObj(app))
if !meta.FinalizerExists(app, onlyRevisionFinalizer) {
meta.AddFinalizer(app, onlyRevisionFinalizer)
klog.InfoS("Register new finalizer for application", "application", klog.KObj(app), "finalizer", onlyRevisionFinalizer)
return true, errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)
}
}
} else {
if meta.FinalizerExists(app, legacyResourceTrackerFinalizer) {
// TODO(roywang) legacyResourceTrackerFinalizer will be deprecated in the future
@@ -274,19 +265,7 @@ func (r *Reconciler) handleFinalizers(ctx context.Context, app *v1beta1.Applicat
meta.RemoveFinalizer(app, legacyResourceTrackerFinalizer)
return true, errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)
}
if meta.FinalizerExists(app, resourceTrackerFinalizer) {
if app.Status.LatestRevision != nil && len(app.Status.LatestRevision.Name) != 0 {
latestTracker := &v1beta1.ResourceTracker{}
latestTracker.SetName(dispatch.ConstructResourceTrackerName(app.Status.LatestRevision.Name, app.Namespace))
if err := r.Client.Delete(ctx, latestTracker); err != nil && !kerrors.IsNotFound(err) {
klog.ErrorS(err, "Failed to delete latest resource tracker", "name", latestTracker.Name)
return true, errors.WithMessage(err, "cannot remove finalizer")
}
}
meta.RemoveFinalizer(app, resourceTrackerFinalizer)
return true, errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)
}
if meta.FinalizerExists(app, onlyRevisionFinalizer) {
if meta.FinalizerExists(app, resourceTrackerFinalizer) || meta.FinalizerExists(app, legacyOnlyRevisionFinalizer) {
listOpts := []client.ListOption{
client.MatchingLabels{
oam.LabelAppName: app.Name,
@@ -303,7 +282,10 @@ func (r *Reconciler) handleFinalizers(ctx context.Context, app *v1beta1.Applicat
return true, errors.WithMessage(err, "cannot remove finalizer")
}
}
meta.RemoveFinalizer(app, onlyRevisionFinalizer)
meta.RemoveFinalizer(app, resourceTrackerFinalizer)
// legacyOnlyRevisionFinalizer will be deprecated in the future
// this is for backward compatibility
meta.RemoveFinalizer(app, legacyOnlyRevisionFinalizer)
return true, errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)
}
}
@@ -50,6 +50,9 @@ var _ = Describe("Test application controller finalizer logic", func() {
ncd := &v1beta1.ComponentDefinition{}
ncdDefJson, _ := yaml.YAMLToJSON([]byte(normalCompDefYaml))
badCD := &v1beta1.ComponentDefinition{}
badCDJson, _ := yaml.YAMLToJSON([]byte(badCompDefYaml))
td := &v1beta1.TraitDefinition{}
tdDefJson, _ := yaml.YAMLToJSON([]byte(crossNsTdYaml))
@@ -69,6 +72,9 @@ var _ = Describe("Test application controller finalizer logic", func() {
Expect(json.Unmarshal(ncdDefJson, ncd)).Should(BeNil())
Expect(k8sClient.Create(ctx, ncd.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
Expect(json.Unmarshal(badCDJson, badCD)).Should(BeNil())
Expect(k8sClient.Create(ctx, badCD.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
})
AfterEach(func() {
@@ -129,6 +135,35 @@ var _ = Describe("Test application controller finalizer logic", func() {
Expect(checkApp.Status.ResourceTracker).Should(BeNil())
})
It("Test error occurs in the middle of dispatching", func() {
appName := "bad-app"
appKey := types.NamespacedName{Namespace: namespace, Name: appName}
app := getApp(appName, namespace, "bad-worker")
Expect(k8sClient.Create(ctx, app)).Should(BeNil())
By("Create a bad workload app")
checkApp := &v1beta1.Application{}
reconcileOnceAfterFinalizer(reconciler, ctrl.Request{NamespacedName: appKey})
Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
// because error occurs in the middle of dispatching
// resource tracker for v1 is created but v1 is not recorded in app status
By("Verify latest app revision is not recorded in status")
Expect(checkApp.Status.LatestRevision).Should(BeNil())
By("Verify ResourceTracker is created")
rt := &v1beta1.ResourceTracker{}
Expect(k8sClient.Get(ctx, getTrackerKey(checkApp.Namespace, checkApp.Name, "v1"), rt)).Should(Succeed())
By("Delete Application")
Expect(k8sClient.Delete(ctx, checkApp)).Should(BeNil())
reconcileOnceAfterFinalizer(reconciler, ctrl.Request{NamespacedName: appKey})
By("Verify ResourceTracker is deleted")
rt = &v1beta1.ResourceTracker{}
Expect(k8sClient.Get(ctx, getTrackerKey(checkApp.Namespace, checkApp.Name, "v1"), rt)).Should(util.NotFoundMatcher{})
})
It("Test cross namespace workload, then delete the app", func() {
appName := "app-2"
appKey := types.NamespacedName{Namespace: namespace, Name: appName}
@@ -400,4 +435,38 @@ spec:
cmd?: [...string]
}
`
badCompDefYaml = `
apiVersion: core.oam.dev/v1beta1
kind: ComponentDefinition
metadata:
name: bad-worker
namespace: vela-system
annotations:
definition.oam.dev/description: "It will make dispatching failed"
spec:
workload:
definition:
apiVersion: apps/v1
kind: Deployment
extension:
template: |
output: {
apiVersion: "apps/v1"
kind: "Deployment"
spec: {
replicas: 0
selector:
matchLabels:
"app.oam.dev/component": context.name
}
}
parameter: {
// +usage=Which image would you like to use for your service
// +short=i
image: string
cmd?: [...string]
}
`
)