remove appContext from app/appRollout controller (#1774)

* refine  assemble and dispatch

Signed-off-by: roy wang <seiwy2010@gmail.com>

* remove app context in app controller

modify clean up app revision

remove old resource tracker related logic

fix unit tests

Signed-off-by: roy wang <seiwy2010@gmail.com>

* fix e2e-test

- get rid of appCtx in test cases
- fix test cases according other logic changes in app controller

remove whole appcontext_test.go file

disable rollout related e2e test provisionally

disable resource tracker related e2e test provisionally

Signed-off-by: roy wang <seiwy2010@gmail.com>

* add finalizer logic for app controller

Signed-off-by: roywang <seiwy2010@gmail.com>

* add new apply option MustBeControllableByAny

make dispatch idempotent

Signed-off-by: roywang <seiwy2010@gmail.com>

* refactor rollout

* fix rollout finalize succeed

Signed-off-by: roywang <seiwy2010@gmail.com>

* add update trait and gc test

fix lint

* fix flaky e2e test

Signed-off-by: roywang <seiwy2010@gmail.com>

* fix comment

* fix comments and add sourceRevision dispatch

delete useless

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

* fix app finalizer backward compatible

Signed-off-by: roywang <seiwy2010@gmail.com>

* fix backward compatability for deprecation of appContext

add unit test for apply option

add e2e test

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

* fix app controller unit test

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

* refine app controller apply logic

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

* fix e2e test of resource tracker

fix e2e test of rollout plan

fix flaky e2e tests

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

* refine comments and remove useless codes

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

* disable appCtx controller

add Component handler into app controller

Signed-off-by: Yue Wang <seiwy2010@gmail.com>

Co-authored-by: wangyike <wangyike.wyk@alibaba-inc.com>
This commit is contained in:
Yue Wang
2021-06-12 15:46:32 +09:00
committed by GitHub
parent 9de6aea5ab
commit 889e38e984
41 changed files with 1983 additions and 2098 deletions

View File

@@ -24,12 +24,14 @@ import (
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/retry"
"k8s.io/klog/v2"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
"github.com/oam-dev/kubevela/pkg/oam"
"github.com/oam-dev/kubevela/pkg/utils/apply"
)
@@ -51,32 +53,33 @@ type AppManifestsDispatcher struct {
applicator apply.Applicator
gcHandler GarbageCollector
appRev *v1beta1.ApplicationRevision
oldRT *v1beta1.ResourceTracker
skipGC bool
appRev *v1beta1.ApplicationRevision
previousRT *v1beta1.ResourceTracker
skipGC bool
appRevName string
namespace string
newRTName string
newRT *v1beta1.ResourceTracker
appRevName string
namespace string
currentRTName string
currentRT *v1beta1.ResourceTracker
}
// EnableGC return an AppManifestsDispatcher that always do GC after dispatching resources.
// GC will calculate diff between the dispatched resouces and ones recorded in the given resource tracker.
func (a *AppManifestsDispatcher) EnableGC(rt *v1beta1.ResourceTracker) *AppManifestsDispatcher {
// EnableUpgradeAndGC return an AppManifestsDispatcher that always do GC after dispatching resources.
// For resources exists in two revision, dispatcher will update their owner to the new resource tracker.
// GC will calculate diff between the dispatched resources and ones recorded in the given resource tracker.
func (a *AppManifestsDispatcher) EnableUpgradeAndGC(rt *v1beta1.ResourceTracker) *AppManifestsDispatcher {
if rt != nil {
a.oldRT = rt.DeepCopy()
a.previousRT = rt.DeepCopy()
}
return a
}
// EnableUpgradeAndSkipGC return an AppManifestsDispatcher that skips GC after dispatching resources.
// For the unchanged resources, dispatcher will update their owner to the newly created resource tracker.
// For resources exists in two revision, dispatcher will update their owner to the new resource tracker.
// It's helpful in a rollout scenario where new revision is going to create a new workload while the old one should not
// be deleted before rollout is terminated.
func (a *AppManifestsDispatcher) EnableUpgradeAndSkipGC(rt *v1beta1.ResourceTracker) *AppManifestsDispatcher {
if rt != nil {
a.oldRT = rt.DeepCopy()
a.previousRT = rt.DeepCopy()
a.skipGC = true
}
return a
@@ -86,7 +89,7 @@ func (a *AppManifestsDispatcher) EnableUpgradeAndSkipGC(rt *v1beta1.ResourceTrac
// If GC is enabled, it will do GC after applying.
// If 'UpgradeAndSkipGC' is enabled, it will:
// - create new resources if not exist before
// - update unchanged resources' owner from the old resource tracker to the new one
// - update unchanged resources' owner from the previous resource tracker to the new one
// - skip deleting(GC) any resources
func (a *AppManifestsDispatcher) Dispatch(ctx context.Context, manifests []*unstructured.Unstructured) (*v1beta1.ResourceTracker, error) {
if err := a.validateAndComplete(ctx); err != nil {
@@ -98,12 +101,12 @@ func (a *AppManifestsDispatcher) Dispatch(ctx context.Context, manifests []*unst
if err := a.applyAndRecordManifests(ctx, manifests); err != nil {
return nil, err
}
if !a.skipGC && a.oldRT != nil && a.oldRT.Name != a.newRTName {
if err := a.gcHandler.GarbageCollect(ctx, a.oldRT, a.newRT); err != nil {
return nil, errors.WithMessagef(err, "cannot do GC based on resource trackers %q and %q", a.oldRT.Name, a.newRTName)
if !a.skipGC && a.previousRT != nil && a.previousRT.Name != a.currentRTName {
if err := a.gcHandler.GarbageCollect(ctx, a.previousRT, a.currentRT); err != nil {
return nil, errors.WithMessagef(err, "cannot do GC based on resource trackers %q and %q", a.previousRT.Name, a.currentRTName)
}
}
return a.newRT.DeepCopy(), nil
return a.currentRT.DeepCopy(), nil
}
// ReferenceScopes add workload reference to scopes' workloadRefPath
@@ -127,61 +130,65 @@ func (a *AppManifestsDispatcher) validateAndComplete(ctx context.Context) error
}
a.appRevName = a.appRev.Name
a.namespace = a.appRev.Namespace
a.newRTName = ConstructResourceTrackerName(a.appRevName, a.namespace)
a.currentRTName = ConstructResourceTrackerName(a.appRevName, a.namespace)
// no matter GC or UpgradeAndSkipGC, it requires a valid and existing resource tracker
if a.oldRT != nil {
existingOldRT := &v1beta1.ResourceTracker{}
if err := a.c.Get(ctx, client.ObjectKey{Name: a.oldRT.Name}, existingOldRT); err != nil {
return errors.Errorf("given resource tracker %q doesn't exist", a.oldRT.Name)
// if upgrade is enbabled (no matter GC or skip GC), it requires a valid existing resource tracker
if a.previousRT != nil && a.previousRT.Name != a.currentRTName {
klog.InfoS("Validate previous resource tracker exists", "previous", klog.KObj(a.previousRT))
gotPreviousRT := &v1beta1.ResourceTracker{}
if err := a.c.Get(ctx, client.ObjectKey{Name: a.previousRT.Name}, gotPreviousRT); err != nil {
return errors.Errorf("given resource tracker %q doesn't exist", a.previousRT.Name)
}
a.oldRT = existingOldRT
a.previousRT = gotPreviousRT
}
klog.InfoS("Given old resource tracker is nil, so skip GC", "appRevision", klog.KObj(a.appRev))
klog.InfoS("Given previous resource tracker is nil or same as current one, so skip GC", "appRevision", klog.KObj(a.appRev))
return nil
}
func (a *AppManifestsDispatcher) createOrGetResourceTracker(ctx context.Context) error {
rt := &v1beta1.ResourceTracker{}
err := a.c.Get(ctx, client.ObjectKey{Name: a.newRTName}, rt)
err := a.c.Get(ctx, client.ObjectKey{Name: a.currentRTName}, rt)
if err == nil {
klog.InfoS("Found a resource tracker matching current app revision", "resourceTracker", rt.Name)
// already exists, no need to update
// because we assume the manifests' references from a specific application revision never change
a.newRT = rt
a.currentRT = rt
return nil
}
if !kerrors.IsNotFound(err) {
return errors.Wrap(err, "cannot get resource tracker")
}
klog.InfoS("Going to create a resource tracker", "resourceTracker", a.newRTName)
rt.SetName(a.newRTName)
klog.InfoS("Going to create a resource tracker", "resourceTracker", a.currentRTName)
rt.SetName(a.currentRTName)
// these labels can help to list resource trackers of a specific application
rt.SetLabels(map[string]string{
oam.LabelAppName: ExtractAppName(a.currentRTName, a.namespace),
oam.LabelAppNamespace: a.namespace,
})
if err := a.c.Create(ctx, rt); err != nil {
klog.ErrorS(err, "Failed to create a resource tracker", "resourceTracker", a.newRTName)
klog.ErrorS(err, "Failed to create a resource tracker", "resourceTracker", a.currentRTName)
return errors.Wrap(err, "cannot create resource tracker")
}
a.newRT = rt
a.currentRT = rt
return nil
}
func (a *AppManifestsDispatcher) applyAndRecordManifests(ctx context.Context, manifests []*unstructured.Unstructured) error {
applyOpts := []apply.ApplyOption{}
if a.oldRT != nil && a.oldRT.Name != a.newRTName {
klog.InfoS("Going to apply and upgrade resources", "from", a.oldRT.Name, "to", a.newRTName)
ctrlUIDs := []types.UID{a.currentRT.UID}
if a.previousRT != nil && a.previousRT.Name != a.currentRTName {
klog.InfoS("Going to apply or upgrade resources", "from", a.previousRT.Name, "to", a.currentRTName)
// if two RT's names are different, it means dispatching operation happens in an upgrade or rollout scenario
// in such two scenarios, for those unchanged manifests, we will
// - check existing resources are controlled by the old resource tracker
// - make sure existing resources are controlled by any of these two resource trackers
// - set new resource tracker as their controller owner
applyOpts = append(applyOpts, apply.MustBeControllableBy(a.oldRT.UID))
} else {
applyOpts = append(applyOpts, apply.MustBeControllableBy(a.newRT.UID))
ctrlUIDs = append(ctrlUIDs, a.previousRT.UID)
}
applyOpts := []apply.ApplyOption{apply.MustBeControllableByAny(ctrlUIDs)}
ownerRef := metav1.OwnerReference{
APIVersion: v1beta1.SchemeGroupVersion.String(),
Kind: reflect.TypeOf(v1beta1.ResourceTracker{}).Name(),
Name: a.newRT.Name,
UID: a.newRT.UID,
Name: a.currentRT.Name,
UID: a.currentRT.UID,
Controller: pointer.BoolPtr(true),
BlockOwnerDeletion: pointer.BoolPtr(true),
}
@@ -201,45 +208,45 @@ func (a *AppManifestsDispatcher) applyAndRecordManifests(ctx context.Context, ma
}
func (a *AppManifestsDispatcher) updateResourceTrackerStatus(ctx context.Context, appliedManifests []*unstructured.Unstructured) error {
// merge applied resources and already recorded ones
trackedResources := []v1beta1.TypedReference{}
// merge applied resources and already tracked ones
if a.currentRT.Status.TrackedResources == nil {
a.currentRT.Status.TrackedResources = make([]v1beta1.TypedReference, 0)
}
for _, rsc := range appliedManifests {
ref := v1beta1.TypedReference{
appliedRef := v1beta1.TypedReference{
APIVersion: rsc.GetAPIVersion(),
Kind: rsc.GetKind(),
Name: rsc.GetName(),
Namespace: rsc.GetNamespace(),
}
alreadyTracked := false
for _, existing := range a.newRT.Status.TrackedResources {
if existing.APIVersion == ref.APIVersion && existing.Kind == ref.Kind &&
existing.Name == ref.Name && existing.Namespace == ref.Namespace {
for _, tracked := range a.currentRT.Status.TrackedResources {
if tracked.APIVersion == appliedRef.APIVersion && tracked.Kind == appliedRef.Kind &&
tracked.Name == appliedRef.Name && tracked.Namespace == appliedRef.Namespace {
alreadyTracked = true
break
}
}
if alreadyTracked {
continue
if !alreadyTracked {
a.currentRT.Status.TrackedResources = append(a.currentRT.Status.TrackedResources, appliedRef)
}
trackedResources = append(trackedResources, ref)
}
a.newRT.Status.TrackedResources = trackedResources
// TODO move TrackedResources from status to spec
// update status with retry
copyRT := a.newRT.DeepCopy()
copyRT := a.currentRT.DeepCopy()
sts := copyRT.Status
if err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
if err = a.c.Get(ctx, client.ObjectKey{Name: a.newRTName}, copyRT); err != nil {
if err = a.c.Get(ctx, client.ObjectKey{Name: a.currentRTName}, copyRT); err != nil {
return
}
copyRT.Status = sts
return a.c.Status().Update(ctx, copyRT)
}); err != nil {
klog.ErrorS(err, "Failed to update resource tracker status", "resourceTracker", a.newRTName)
klog.ErrorS(err, "Failed to update resource tracker status", "resourceTracker", a.currentRTName)
return errors.Wrap(err, "cannot update resource tracker status")
}
klog.InfoS("Successfully update resource tracker status", "resourceTracker", a.newRTName)
klog.InfoS("Successfully update resource tracker status", "resourceTracker", a.currentRTName)
return nil
}

View File

@@ -229,7 +229,7 @@ var _ = Describe("Test AppManifestsDispatcher", func() {
Expect(k8sClient.Get(ctx, client.ObjectKey{Name: pvName1}, &corev1.PersistentVolume{})).Should(Succeed())
By("Dispatch application revision 1 again with v1 as latest RT")
dp = NewAppManifestsDispatcher(k8sClient, appRev1).EnableGC(rtForAppV1)
dp = NewAppManifestsDispatcher(k8sClient, appRev1).EnableUpgradeAndGC(rtForAppV1)
_, err = dp.Dispatch(ctx, []*unstructured.Unstructured{deploy1, svc1, pv1})
Expect(err).Should(BeNil())
By("Verify resources still exist")
@@ -250,7 +250,7 @@ var _ = Describe("Test AppManifestsDispatcher", func() {
Expect(k8sClient.Get(ctx, client.ObjectKey{Name: pvName1}, &corev1.PersistentVolume{})).Should(Succeed())
By("Dispatch application revision 2 with v1 as latest RT")
dp2 := NewAppManifestsDispatcher(k8sClient, appRev2).EnableGC(rtForAppV1)
dp2 := NewAppManifestsDispatcher(k8sClient, appRev2).EnableUpgradeAndGC(rtForAppV1)
_, err = dp2.Dispatch(ctx, []*unstructured.Unstructured{deploy2, svc2, pv2})
Expect(err).Should(BeNil())
By("Verify v2 resources are applied successfully")
@@ -284,7 +284,7 @@ var _ = Describe("Test AppManifestsDispatcher", func() {
Expect(owner.Name).Should(Equal(rtForAppV1.Name))
By("Dispatch application revision 2 with v1 as latest RT")
dp2 := NewAppManifestsDispatcher(k8sClient, appRev2).EnableGC(rtForAppV1)
dp2 := NewAppManifestsDispatcher(k8sClient, appRev2).EnableUpgradeAndGC(rtForAppV1)
rtForAppV2, err := dp2.Dispatch(ctx, []*unstructured.Unstructured{deploy2, svc1, pv2}) // manifests have 'svc1'
Expect(err).Should(BeNil())

View File

@@ -93,8 +93,8 @@ func (h *GCHandler) validate() error {
oldRTName := h.oldRT.Name
newRTName := h.newRT.Name
if strings.HasSuffix(oldRTName, h.namespace) && strings.HasSuffix(newRTName, h.namespace) {
if extractAppNameFromResourceTrackerName(oldRTName, h.namespace) ==
extractAppNameFromResourceTrackerName(newRTName, h.namespace) {
if ExtractAppName(oldRTName, h.namespace) ==
ExtractAppName(newRTName, h.namespace) {
return nil
}
}

View File

@@ -28,7 +28,13 @@ func ConstructResourceTrackerName(appRevName, ns string) string {
return fmt.Sprintf("%s-%s", appRevName, ns)
}
func extractAppNameFromResourceTrackerName(name, ns string) string {
splits := strings.Split(strings.TrimSuffix(name, "-"+ns), "-")
// ExtractAppName get application name from resource tracker name
func ExtractAppName(resourceTrackerName, ns string) string {
splits := strings.Split(strings.TrimSuffix(resourceTrackerName, "-"+ns), "-")
return strings.Join(splits[0:len(splits)-1], "-")
}
// ExtractAppRevisionName get application revision name from resource tracker name
func ExtractAppRevisionName(resourceTrackerName, ns string) string {
return strings.TrimSuffix(resourceTrackerName, "-"+ns)
}

View File

@@ -41,12 +41,16 @@ func TestExtractAppNameFromResourceTrackerName(t *testing.T) {
for _, tc := range testcases {
gotRTName := ConstructResourceTrackerName(tc.appRevName, tc.ns)
gotAppName := extractAppNameFromResourceTrackerName(gotRTName, tc.ns)
gotAppName := ExtractAppName(gotRTName, tc.ns)
gotAppRevName := ExtractAppRevisionName(gotRTName, tc.ns)
if gotRTName != tc.wantRTName {
t.Fatalf("expect resource tracker name %q but got %q", tc.wantRTName, gotRTName)
}
if gotAppName != tc.wantAppName {
t.Fatalf("expect app name %q but got %q", tc.wantAppName, gotAppName)
}
if gotAppRevName != tc.appRevName {
t.Fatalf("expect app revision name %q but got %q", tc.appRevName, gotAppRevName)
}
}
}