fix apply only once observedGeneration should mark after meet all dependency requirements && add log for apply only once

This commit is contained in:
天元
2021-02-24 17:31:31 +08:00
parent 309786338a
commit 580110ed44
8 changed files with 147 additions and 41 deletions
+4 -1
View File
@@ -126,7 +126,10 @@ func main() {
setupLog.Info(fmt.Sprintf("KubeVela Version: %s, GIT Revision: %s.", version.VelaVersion, version.GitRevision))
setupLog.Info(fmt.Sprintf("Disable Capabilities: %s.", disableCaps))
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
restConfig := ctrl.GetConfigOrDie()
restConfig.UserAgent = kubevelaName + "/" + version.GitRevision
mgr, err := ctrl.NewManager(restConfig, ctrl.Options{
Scheme: scheme,
MetricsBindAddress: metricsAddr,
LeaderElection: enableLeaderElection,
@@ -103,7 +103,7 @@ func Setup(mgr ctrl.Manager, args core.Args, l logging.Logger) error {
CustomRevisionHookURL: args.CustomRevisionHookURL,
}).
Complete(NewReconciler(mgr, dm,
WithLogger(l.WithValues("controller", name)),
l.WithValues("controller", name),
WithRecorder(event.NewAPIRecorder(mgr.GetEventRecorderFor(name))),
WithApplyOnceOnlyMode(args.ApplyMode)))
}
@@ -149,13 +149,6 @@ func WithGarbageCollector(gc GarbageCollector) ReconcilerOption {
}
}
// WithLogger specifies how the Reconciler should log messages.
func WithLogger(l logging.Logger) ReconcilerOption {
return func(r *OAMApplicationReconciler) {
r.log = l
}
}
// WithRecorder specifies how the Reconciler should record events.
func WithRecorder(er event.Recorder) ReconcilerOption {
return func(r *OAMApplicationReconciler) {
@@ -187,7 +180,7 @@ func WithApplyOnceOnlyMode(mode core.ApplyOnceOnlyMode) ReconcilerOption {
// NewReconciler returns an OAMApplicationReconciler that reconciles ApplicationConfigurations
// by rendering and instantiating their Components and Traits.
func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, o ...ReconcilerOption) *OAMApplicationReconciler {
func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, log logging.Logger, o ...ReconcilerOption) *OAMApplicationReconciler {
r := &OAMApplicationReconciler{
client: m.GetClient(),
scheme: m.GetScheme(),
@@ -199,12 +192,12 @@ func NewReconciler(m ctrl.Manager, dm discoverymapper.DiscoveryMapper, o ...Reco
trait: ResourceRenderFn(renderTrait),
},
workloads: &workloads{
applicator: apply.NewAPIApplicator(m.GetClient()),
applicator: apply.NewAPIApplicator(m.GetClient(), log),
rawClient: m.GetClient(),
dm: dm,
},
gc: GarbageCollectorFn(eligible),
log: logging.NewNopLogger(),
log: log,
record: event.NewNopRecorder(),
preHooks: make(map[string]ControllerHooks),
postHooks: make(map[string]ControllerHooks),
@@ -326,9 +319,9 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco
log.Debug("Successfully rendered components", "workloads", len(workloads))
r.record.Event(ac, event.Normal(reasonRenderComponents, "Successfully rendered components", "workloads", strconv.Itoa(len(workloads))))
applyOpts := []apply.ApplyOption{apply.MustBeControllableBy(ac.GetUID()), applyOnceOnly(ac, r.applyOnceOnlyMode)}
applyOpts := []apply.ApplyOption{apply.MustBeControllableBy(ac.GetUID()), applyOnceOnly(ac, r.applyOnceOnlyMode, log)}
if err := r.workloads.Apply(ctx, ac.Status.Workloads, workloads, firstReconcile, newComponents, applyOpts...); err != nil {
log.Debug("Cannot apply workload", "error", err, "requeue-after", time.Now().Add(shortWait))
log.Debug("Cannot apply components", "error", err, "requeue-after", time.Now().Add(shortWait))
r.record.Event(ac, event.Warning(reasonCannotApplyComponents, err))
ac.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errApplyComponents)))
return errResult, errors.Wrap(r.UpdateStatus(ctx, ac), errUpdateAppConfigStatus)
@@ -388,7 +381,6 @@ func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac, acPatch
historyWorkloads := make([]v1alpha2.HistoryWorkload, 0)
for i, w := range workloads {
ac.Status.Workloads[i] = workloads[i].Status()
ac.Status.Workloads[i].ObservedGeneration = ac.GetGeneration()
if !w.RevisionEnabled {
continue
}
@@ -425,6 +417,12 @@ func updateObservedGeneration(ac *v1alpha2.ApplicationConfiguration) {
if ac.Status.ObservedGeneration != ac.Generation {
ac.Status.ObservedGeneration = ac.Generation
}
for i, w := range ac.Status.Workloads {
// only all workload meet requirements can say the generation is observed successfully
if w.ObservedGeneration != ac.Generation && len(ac.Status.Dependency.Unsatisfied) == 0 {
ac.Status.Workloads[i].ObservedGeneration = ac.GetGeneration()
}
}
}
func patchExtraStatusField(acStatus *v1alpha2.ApplicationConfigurationStatus, acPatchStatus v1alpha2.ApplicationConfigurationStatus) {
@@ -630,12 +628,11 @@ func (e *GenerationUnchanged) Error() string {
// applyOnceOnly is an ApplyOption that controls the applying mechanism for workload and trait.
// More detail refers to the ApplyOnceOnlyMode type annotation
func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnlyMode) apply.ApplyOption {
func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnlyMode, log logging.Logger) apply.ApplyOption {
return func(_ context.Context, existing, desired runtime.Object) error {
if mode == core.ApplyOnceOnlyOff {
return nil
}
d, _ := desired.(metav1.Object)
if d == nil {
return errors.Errorf("cannot access metadata of object being applied: %q",
@@ -647,6 +644,7 @@ func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnl
dLabels[oam.LabelOAMResourceType] != oam.ResourceTypeTrait {
// this ApplyOption only works for workload and trait
// skip if the resource is not workload nor trait, e.g., scope
log.Info("ignore apply only once check, because resourceType is not workload or trait", oam.LabelOAMResourceType, dLabels[oam.LabelOAMResourceType])
return nil
}
@@ -654,6 +652,7 @@ func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnl
if existing == nil {
if mode != core.ApplyOnceOnlyForce {
// non-force mode will always create the resource if not exist.
log.Info("apply only once with mode:" + string(mode) + ", but old resource not exist, will create a new one")
return nil
}
@@ -677,11 +676,14 @@ func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnl
}
// don't use if-else here because it will miss the case that the resource is a trait
if createdBefore {
// the resource was created before and appconfig status recorded the resource version applied
// if recored ObservedGeneration and ComponentRevisionName both equal to the applied resource's,
// the resource was created before and appConfig status recorded the resource version applied
// if recorded ObservedGeneration and ComponentRevisionName both equal to the applied resource's,
// that means its spec is not changed
if (strconv.Itoa(int(w.ObservedGeneration)) != dAnnots[oam.AnnotationAppGeneration]) ||
(w.ComponentRevisionName != dLabels[oam.LabelAppComponentRevision]) {
log.Info("apply only once with mode: "+string(mode)+", but condition not meet, will create new",
oam.AnnotationAppGeneration, strconv.Itoa(int(w.ObservedGeneration))+"/"+dAnnots[oam.AnnotationAppGeneration],
oam.LabelAppComponentRevision, w.ComponentRevisionName+"/"+dLabels[oam.LabelAppComponentRevision])
// its spec is changed, so re-create the resource
return nil
}
@@ -689,6 +691,7 @@ func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnl
return &GenerationUnchanged{}
}
}
log.Info("apply only once with mode: force, but resource not created before, will create new", "appConfig", ac.Name)
// no recorded workloads nor traits matches the applied resource
// that means the resource is not created before, so create it
return nil
@@ -701,10 +704,13 @@ func applyOnceOnly(ac *v1alpha2.ApplicationConfiguration, mode core.ApplyOnceOnl
existing.GetObjectKind().GroupVersionKind())
}
eLabels := e.GetLabels()
// if existing reource's (observed)AppConfigGeneration and ComponentRevisionName both equal to the applied one's,
// if existing resource's (observed)AppConfigGeneration and ComponentRevisionName both equal to the applied one's,
// that means its spec is not changed
if (e.GetAnnotations()[oam.AnnotationAppGeneration] != dAnnots[oam.AnnotationAppGeneration]) ||
(eLabels[oam.LabelAppComponentRevision] != dLabels[oam.LabelAppComponentRevision]) {
log.Info("apply only once with mode: "+string(mode)+", but new generation or revision created, will create new",
oam.AnnotationAppGeneration, e.GetAnnotations()[oam.AnnotationAppGeneration]+"/"+dAnnots[oam.AnnotationAppGeneration],
oam.LabelAppComponentRevision, eLabels[oam.LabelAppComponentRevision]+"/"+dLabels[oam.LabelAppComponentRevision])
// its spec is changed, so apply new configuration to it
return nil
}
@@ -666,7 +666,7 @@ func TestReconciler(t *testing.T) {
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
r := NewReconciler(tc.args.m, nil, tc.args.o...)
r := NewReconciler(tc.args.m, nil, logging.NewNopLogger(), tc.args.o...)
got, err := r.Reconcile(reconcile.Request{})
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
@@ -1870,7 +1870,7 @@ func TestUpdateStatus(t *testing.T) {
},
}
r := NewReconciler(m, nil)
r := NewReconciler(m, nil, logging.NewNopLogger())
ac := &v1alpha2.ApplicationConfiguration{}
err := r.client.Get(context.Background(), types.NamespacedName{Name: "example-appconfig"}, ac)
@@ -4,6 +4,8 @@ import (
"context"
"time"
"k8s.io/apimachinery/pkg/types"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
@@ -288,6 +290,77 @@ var _ = Describe("Test apply (workloads/traits) once only", func() {
})
When("ApplyOnceOnlyForce is enabled", func() {
It("should normally create workload/trait resources at fist time", func() {
By("Enable ApplyOnceOnlyForce")
reconciler.applyOnceOnlyMode = core.ApplyOnceOnlyForce
component2 := v1alpha2.Component{
TypeMeta: metav1.TypeMeta{
APIVersion: "core.oam.dev/v1alpha2",
Kind: "Component",
},
ObjectMeta: metav1.ObjectMeta{
Name: "mycomp2",
Namespace: namespace,
},
Spec: v1alpha2.ComponentSpec{
Workload: runtime.RawExtension{
Object: &cw,
},
},
}
newFakeTrait := fakeTrait.DeepCopy()
newFakeTrait.SetName("mytrait2")
appConfig2 := v1alpha2.ApplicationConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "myac2",
Namespace: namespace,
},
Spec: v1alpha2.ApplicationConfigurationSpec{
Components: []v1alpha2.ApplicationConfigurationComponent{
{
ComponentName: "mycomp2",
Traits: []v1alpha2.ComponentTrait{
{Trait: runtime.RawExtension{Object: newFakeTrait}},
},
},
},
},
}
By("Create Component")
Expect(k8sClient.Create(ctx, &component2)).Should(Succeed())
time.Sleep(time.Second)
By("Creat appConfig & check successfully")
Expect(k8sClient.Create(ctx, &appConfig2)).Should(Succeed())
time.Sleep(time.Second)
By("Reconcile")
Expect(func() error {
_, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: "myac2", Namespace: namespace}})
return err
}()).Should(BeNil())
time.Sleep(2 * time.Second)
By("Get workload instance & Check workload spec")
cwObj := v1alpha2.ContainerizedWorkload{}
Eventually(func() error {
return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "mycomp2"}, &cwObj)
}, 5*time.Second, time.Second).Should(BeNil())
Expect(cwObj.Spec.Containers[0].Image).Should(Equal(image1))
By("Get trait instance & Check trait spec")
fooObj := &unstructured.Unstructured{}
fooObj.SetAPIVersion("example.com/v1")
fooObj.SetKind("Foo")
Eventually(func() error {
return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "mytrait2"}, fooObj)
}, 3*time.Second, time.Second).Should(BeNil())
fooObjV, _, _ := unstructured.NestedString(fooObj.Object, "spec", "key")
Expect(fooObjV).Should(Equal(traitSpecValue1))
})
It("should not revert changes of workload/trait made by others", func() {
By("Enable ApplyOnceOnlyForce")
reconciler.applyOnceOnlyMode = core.ApplyOnceOnlyForce
@@ -433,7 +506,7 @@ var _ = Describe("Test apply (workloads/traits) once only", func() {
recreatedFooObj.SetKind("Foo")
Expect(k8sClient.Get(ctx, traitObjKey, &recreatedFooObj)).Should(util.NotFoundMatcher{})
By("Update Appconfig to trigger generation augment")
By("Update AppConfig to trigger generation updated")
unstructured.SetNestedField(fakeTrait.Object, "newvalue", "spec", "key")
appConfig = v1alpha2.ApplicationConfiguration{
ObjectMeta: metav1.ObjectMeta{
@@ -452,6 +525,7 @@ var _ = Describe("Test apply (workloads/traits) once only", func() {
},
}
Expect(k8sClient.Patch(ctx, &appConfig, client.Merge)).Should(Succeed())
time.Sleep(1 * time.Second)
By("Check AppConfig is updated successfully")
updateAC := v1alpha2.ApplicationConfiguration{}
@@ -461,6 +535,9 @@ var _ = Describe("Test apply (workloads/traits) once only", func() {
}
return updateAC.GetGeneration()
}, 3*time.Second, time.Second).Should(Equal(int64(2)))
By("Reconcile")
reconcileRetry(reconciler, req)
time.Sleep(2 * time.Second)
By("Check workload is re-created by reconciliation")
Eventually(func() error {
@@ -150,7 +150,7 @@ var _ = BeforeSuite(func(done Done) {
}, time.Second*30, time.Millisecond*500).Should(BeNil())
Expect(mapping.Resource.Resource).Should(Equal("foo"))
reconciler = NewReconciler(mgr, dm, WithLogger(logging.NewLogrLogger(ctrl.Log.WithName("suit-test-appconfig"))))
reconciler = NewReconciler(mgr, dm, logging.NewLogrLogger(ctrl.Log.WithName("suit-test-appconfig")))
componentHandler = &ComponentHandler{Client: k8sClient, RevisionLimit: 100, Logger: logging.NewLogrLogger(ctrl.Log.WithName("component-handler"))}
By("Creating workload definition and trait definition")
+30 -13
View File
@@ -3,22 +3,23 @@ package apply
import (
"context"
"github.com/crossplane/crossplane-runtime/pkg/logging"
"github.com/pkg/errors"
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/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"github.com/oam-dev/kubevela/pkg/oam"
)
// Applicator applies new state to an object or create it if not exist.
// It employes the same mechanism as `kubectl apply`, that is, for each resource being applied,
// It uses the same mechanism as `kubectl apply`, that is, for each resource being applied,
// computing a three-way diff merge in client side based on its current state, modified stated,
// and last-applied-state which is tracked through an specific annotaion.
// and last-applied-state which is tracked through an specific annotation.
// If the resource doesn't exist before, Apply will create it.
type Applicator interface {
Apply(context.Context, runtime.Object, ...ApplyOption) error
@@ -32,20 +33,23 @@ type ApplyOption func(ctx context.Context, existing, desired runtime.Object) err
// NewAPIApplicator creates an Applicator that applies state to an
// object or creates the object if not exist.
func NewAPIApplicator(c client.Client) *APIApplicator {
func NewAPIApplicator(c client.Client, log logging.Logger) *APIApplicator {
return &APIApplicator{
creatorFn(createOrGetExisting),
patcherFn(threeWayMergePatch), c}
creator: creatorFn(createOrGetExisting),
patcher: patcherFn(threeWayMergePatch),
c: c,
log: log,
}
}
type creator interface {
createOrGetExisting(context.Context, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error)
createOrGetExisting(context.Context, logging.Logger, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error)
}
type creatorFn func(context.Context, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error)
type creatorFn func(context.Context, logging.Logger, client.Client, runtime.Object, ...ApplyOption) (runtime.Object, error)
func (fn creatorFn) createOrGetExisting(ctx context.Context, c client.Client, o runtime.Object, ao ...ApplyOption) (runtime.Object, error) {
return fn(ctx, c, o, ao...)
func (fn creatorFn) createOrGetExisting(ctx context.Context, log logging.Logger, c client.Client, o runtime.Object, ao ...ApplyOption) (runtime.Object, error) {
return fn(ctx, log, c, o, ao...)
}
type patcher interface {
@@ -62,12 +66,23 @@ func (fn patcherFn) patch(c, m runtime.Object) (client.Patch, error) {
type APIApplicator struct {
creator
patcher
c client.Client
c client.Client
log logging.Logger
}
// loggingApply will record a log with desired object applied
func loggingApply(log logging.Logger, msg string, desired runtime.Object) {
d, ok := desired.(metav1.Object)
if !ok {
log.Debug(msg, "resource", desired.GetObjectKind().GroupVersionKind().String())
return
}
log.Debug(msg, "name", d.GetName(), "resource", desired.GetObjectKind().GroupVersionKind().String())
}
// Apply applies new state to an object or create it if not exist
func (a *APIApplicator) Apply(ctx context.Context, desired runtime.Object, ao ...ApplyOption) error {
existing, err := a.createOrGetExisting(ctx, a.c, desired, ao...)
existing, err := a.createOrGetExisting(ctx, a.log, a.c, desired, ao...)
if err != nil {
return err
}
@@ -79,6 +94,7 @@ func (a *APIApplicator) Apply(ctx context.Context, desired runtime.Object, ao ..
if err := executeApplyOptions(ctx, existing, desired, ao); err != nil {
return err
}
loggingApply(a.log, "patching object", desired)
patch, err := a.patcher.patch(existing, desired)
if err != nil {
return errors.Wrap(err, "cannot calculate patch by computing a three way diff")
@@ -88,7 +104,7 @@ func (a *APIApplicator) Apply(ctx context.Context, desired runtime.Object, ao ..
// createOrGetExisting will create the object if it does not exist
// or get and return the existing object
func createOrGetExisting(ctx context.Context, c client.Client, desired runtime.Object, ao ...ApplyOption) (runtime.Object, error) {
func createOrGetExisting(ctx context.Context, log logging.Logger, c client.Client, desired runtime.Object, ao ...ApplyOption) (runtime.Object, error) {
m, ok := desired.(oam.Object)
if !ok {
return nil, errors.New("cannot access object metadata")
@@ -102,6 +118,7 @@ func createOrGetExisting(ctx context.Context, c client.Client, desired runtime.O
if err := addLastAppliedConfigAnnotation(desired); err != nil {
return nil, err
}
loggingApply(log, "creating object", desired)
return nil, errors.Wrap(c.Create(ctx, desired), "cannot create object")
}
+2 -1
View File
@@ -11,6 +11,7 @@ import (
oamcore "github.com/oam-dev/kubevela/apis/core.oam.dev"
oamstd "github.com/oam-dev/kubevela/apis/standard.oam.dev/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/logging"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
@@ -56,7 +57,7 @@ var _ = BeforeSuite(func(done Done) {
rawClient, err = client.New(cfg, client.Options{Scheme: testScheme})
Expect(err).ShouldNot(HaveOccurred())
Expect(rawClient).ShouldNot(BeNil())
k8sApplicator = NewAPIApplicator(rawClient)
k8sApplicator = NewAPIApplicator(rawClient, logging.NewNopLogger())
By("Create test namespace")
applyNS = corev1.Namespace{
+5 -3
View File
@@ -4,6 +4,7 @@ import (
"context"
"testing"
"github.com/crossplane/crossplane-runtime/pkg/logging"
"github.com/crossplane/crossplane-runtime/pkg/test"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
@@ -118,13 +119,14 @@ func TestAPIApplicator(t *testing.T) {
for caseName, tc := range cases {
t.Run(caseName, func(t *testing.T) {
a := &APIApplicator{
creator: creatorFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...ApplyOption) (runtime.Object, error) {
creator: creatorFn(func(_ context.Context, _ logging.Logger, _ client.Client, _ runtime.Object, _ ...ApplyOption) (runtime.Object, error) {
return tc.args.existing, tc.args.creatorErr
}),
patcher: patcherFn(func(c, m runtime.Object) (client.Patch, error) {
return nil, tc.args.patcherErr
}),
c: tc.c,
c: tc.c,
log: logging.NewNopLogger(),
}
result := a.Apply(ctx, tc.args.desired, tc.args.ao...)
if diff := cmp.Diff(tc.want, result, test.EquateErrors()); diff != "" {
@@ -285,7 +287,7 @@ func TestCreator(t *testing.T) {
for caseName, tc := range cases {
t.Run(caseName, func(t *testing.T) {
result, err := createOrGetExisting(ctx, tc.c, tc.args.desired, tc.args.ao...)
result, err := createOrGetExisting(ctx, logging.NewNopLogger(), tc.c, tc.args.desired, tc.args.ao...)
if diff := cmp.Diff(tc.want.existing, result); diff != "" {
t.Errorf("\n%s\ncreateOrGetExisting(...): -want , +got \n%s\n", tc.reason, diff)
}