diff --git a/pkg/controllers/collaset/revision.go b/pkg/controllers/collaset/revision.go index 3f58c730..8877fb47 100644 --- a/pkg/controllers/collaset/revision.go +++ b/pkg/controllers/collaset/revision.go @@ -78,16 +78,6 @@ func (roa *revisionOwnerAdapter) GetPatch(obj metav1.Object) ([]byte, error) { return getCollaSetPatch(cs) } -func (roa *revisionOwnerAdapter) GetSelectorLabels(obj metav1.Object) map[string]string { - ips, _ := obj.(*appsalphav1.CollaSet) - labels := map[string]string{} - for k, v := range ips.Spec.Template.Labels { - labels[k] = v - } - - return labels -} - func (roa *revisionOwnerAdapter) GetCurrentRevision(obj metav1.Object) string { ips, _ := obj.(*appsalphav1.CollaSet) return ips.Status.CurrentRevision diff --git a/pkg/controllers/utils/revision/revision_manager.go b/pkg/controllers/utils/revision/revision_manager.go index 7a79c630..10a0a354 100644 --- a/pkg/controllers/utils/revision/revision_manager.go +++ b/pkg/controllers/utils/revision/revision_manager.go @@ -46,7 +46,6 @@ type OwnerAdapter interface { GetCollisionCount(obj metav1.Object) *int32 GetHistoryLimit(obj metav1.Object) int32 GetPatch(obj metav1.Object) ([]byte, error) - GetSelectorLabels(obj metav1.Object) map[string]string GetCurrentRevision(obj metav1.Object) string IsInUsed(obj metav1.Object, controllerRevision string) bool } @@ -287,11 +286,7 @@ func (rm *RevisionManager) newRevision(set metav1.Object, revision int64, collis return nil, err } - revisionLabels := rm.ownerGetter.GetSelectorLabels(set) - if revisionLabels == nil { - revisionLabels = map[string]string{} - } - + revisionLabels := map[string]string{} if selector := rm.ownerGetter.GetSelector(set); selector != nil { for k, v := range selector.MatchLabels { revisionLabels[k] = v @@ -309,16 +304,6 @@ func (rm *RevisionManager) newRevision(set metav1.Object, revision int64, collis } cr.Namespace = set.GetNamespace() - if cr.ObjectMeta.Annotations == nil { - cr.ObjectMeta.Annotations = make(map[string]string) - } - for key, value := range set.GetAnnotations() { - cr.ObjectMeta.Annotations[key] = value - } - - if cr.ObjectMeta.Labels == nil { - cr.ObjectMeta.Labels = make(map[string]string) - } return cr, nil } diff --git a/pkg/controllers/utils/revision/revision_manager_test.go b/pkg/controllers/utils/revision/revision_manager_test.go index ce12b896..2dffce23 100644 --- a/pkg/controllers/utils/revision/revision_manager_test.go +++ b/pkg/controllers/utils/revision/revision_manager_test.go @@ -20,193 +20,259 @@ import ( "context" "encoding/json" "fmt" - "os" - "path/filepath" - "strings" "testing" "time" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "kusionstack.io/operating/apis" - "kusionstack.io/operating/pkg/utils/inject" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) var ( - env *envtest.Environment - mgr manager.Manager - request chan reconcile.Request + c client.Client + g *WithT - ctx context.Context - cancel context.CancelFunc - c client.Client + selectedLabels = map[string]string{"test": "foo"} + selector, _ = metav1.ParseToLabelSelector("test=foo") ) -var _ = Describe("revision manager", func() { +func TestRevisionConstruction(t *testing.T) { + g = NewGomegaWithT(t) + testcase := "test-revision-construction" + schema := runtime.NewScheme() + corev1.AddToScheme(schema) + appsv1.AddToScheme(schema) + c = fake.NewClientBuilder().WithScheme(schema).Build() + g.Expect(createNamespace(c, testcase)).Should(BeNil()) - selectedLabels := map[string]string{"test": "foo"} - selector, err := metav1.ParseToLabelSelector("test=foo") - Expect(err).Should(BeNil()) - - It("revision construction", func() { - testcase := "test-revision-construction" - Expect(createNamespace(c, testcase)).Should(BeNil()) - - deploy := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testcase, - Name: testcase, - }, - Spec: appsv1.DeploymentSpec{ - Selector: selector, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: selectedLabels, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test", - Image: "nginx:v1", - }, + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testcase, + Name: testcase, + }, + Spec: appsv1.DeploymentSpec{ + Selector: selector, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: selectedLabels, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "nginx:v1", }, }, }, }, - } - Expect(mgr.GetClient().Create(context.TODO(), deploy)).Should(BeNil()) - - adapter := &OwnerAdapterImpl{ - name: testcase, - selector: selector, - selectedLabels: selectedLabels, - collisionCount: 0, - historyLimit: 2, - currentRevision: "test", - inUsed: true, - } + }, + } + g.Expect(c.Create(context.TODO(), deploy)).Should(BeNil()) + + adapter := &OwnerAdapterImpl{ + name: testcase, + selector: selector, + selectedLabels: selectedLabels, + collisionCount: 0, + historyLimit: 2, + currentRevision: "test", + inUsed: true, + } - revisionManager := NewRevisionManager(mgr.GetClient(), mgr.GetScheme(), adapter) - currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err := revisionManager.ConstructRevisions(deploy, false) - Expect(err).Should(BeNil()) - Expect(createNewRevision).Should(BeTrue()) - Expect(collisionCount).ShouldNot(BeNil()) - Expect(*collisionCount).Should(BeEquivalentTo(0)) - Expect(len(revisionList)).Should(BeEquivalentTo(1)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(updatedRevision.Name).Should(BeEquivalentTo(currentRevision.Name)) - v1RevisionName := updatedRevision.Name - waitingCacheUpdate(deploy.Namespace, 1) - adapter.currentRevision = updatedRevision.Name - - // updating deploy spec should construct a new updated CcontrollerRevision - deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v2" - currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err = revisionManager.ConstructRevisions(deploy, false) - Expect(err).Should(BeNil()) - Expect(createNewRevision).Should(BeTrue()) - Expect(collisionCount).ShouldNot(BeNil()) - Expect(*collisionCount).Should(BeEquivalentTo(0)) - Expect(len(revisionList)).Should(BeEquivalentTo(2)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(updatedRevision.Name).ShouldNot(BeEquivalentTo(currentRevision.Name)) - waitingCacheUpdate(deploy.Namespace, 2) - - // reconcile with same spec with current revision is updated to updated revision - adapter.currentRevision = updatedRevision.Name - currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err = revisionManager.ConstructRevisions(deploy, false) - Expect(err).Should(BeNil()) - Expect(createNewRevision).Should(BeFalse()) - Expect(collisionCount).ShouldNot(BeNil()) - Expect(*collisionCount).Should(BeEquivalentTo(0)) - Expect(len(revisionList)).Should(BeEquivalentTo(2)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(updatedRevision.Name).Should(BeEquivalentTo(currentRevision.Name)) - waitingCacheUpdate(deploy.Namespace, 2) - - // updating deploy spec to old version should not construct a new updated CcontrollerRevision - deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v1" - currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err = revisionManager.ConstructRevisions(deploy, false) - Expect(err).Should(BeNil()) - Expect(createNewRevision).Should(BeFalse()) - Expect(collisionCount).ShouldNot(BeNil()) - Expect(*collisionCount).Should(BeEquivalentTo(0)) - Expect(len(revisionList)).Should(BeEquivalentTo(2)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) - Expect(updatedRevision.Name).ShouldNot(BeEquivalentTo(currentRevision.Name)) - Expect(updatedRevision.Name).Should(BeEquivalentTo(v1RevisionName)) - }) - - It("revision cleanup", func() { - testcase := "test-revision-cleanup" - Expect(createNamespace(c, testcase)).Should(BeNil()) - - deploy := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testcase, - Name: testcase, - }, - Spec: appsv1.DeploymentSpec{ - Selector: selector, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: selectedLabels, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test", - Image: "nginx:v1", - }, + revisionManager := NewRevisionManager(c, schema, adapter) + currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err := revisionManager.ConstructRevisions(deploy, false) + g.Expect(err).Should(BeNil()) + g.Expect(createNewRevision).Should(BeTrue()) + g.Expect(collisionCount).ShouldNot(BeNil()) + g.Expect(*collisionCount).Should(BeEquivalentTo(0)) + g.Expect(len(revisionList)).Should(BeEquivalentTo(1)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(updatedRevision.Name).Should(BeEquivalentTo(currentRevision.Name)) + v1RevisionName := updatedRevision.Name + waitingCacheUpdate(deploy.Namespace, 1) + adapter.currentRevision = updatedRevision.Name + + // updating deploy spec should construct a new updated CcontrollerRevision + deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v2" + currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err = revisionManager.ConstructRevisions(deploy, false) + g.Expect(err).Should(BeNil()) + g.Expect(createNewRevision).Should(BeTrue()) + g.Expect(collisionCount).ShouldNot(BeNil()) + g.Expect(*collisionCount).Should(BeEquivalentTo(0)) + g.Expect(len(revisionList)).Should(BeEquivalentTo(2)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(updatedRevision.Name).ShouldNot(BeEquivalentTo(currentRevision.Name)) + waitingCacheUpdate(deploy.Namespace, 2) + + // reconcile with same spec with current revision is updated to updated revision + adapter.currentRevision = updatedRevision.Name + currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err = revisionManager.ConstructRevisions(deploy, false) + g.Expect(err).Should(BeNil()) + g.Expect(createNewRevision).Should(BeFalse()) + g.Expect(collisionCount).ShouldNot(BeNil()) + g.Expect(*collisionCount).Should(BeEquivalentTo(0)) + g.Expect(len(revisionList)).Should(BeEquivalentTo(2)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(updatedRevision.Name).Should(BeEquivalentTo(currentRevision.Name)) + waitingCacheUpdate(deploy.Namespace, 2) + + // updating deploy spec to old version should not construct a new updated CcontrollerRevision + deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v1" + currentRevision, updatedRevision, revisionList, collisionCount, createNewRevision, err = revisionManager.ConstructRevisions(deploy, false) + g.Expect(err).Should(BeNil()) + g.Expect(createNewRevision).Should(BeFalse()) + g.Expect(collisionCount).ShouldNot(BeNil()) + g.Expect(*collisionCount).Should(BeEquivalentTo(0)) + g.Expect(len(revisionList)).Should(BeEquivalentTo(2)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(len(currentRevision.Name)).ShouldNot(BeEquivalentTo(0)) + g.Expect(updatedRevision.Name).ShouldNot(BeEquivalentTo(currentRevision.Name)) + g.Expect(updatedRevision.Name).Should(BeEquivalentTo(v1RevisionName)) +} + +func TestRevisionCleanUp(t *testing.T) { + g = NewGomegaWithT(t) + testcase := "test-revision-cleanup" + schema := runtime.NewScheme() + corev1.AddToScheme(schema) + appsv1.AddToScheme(schema) + c = fake.NewClientBuilder().WithScheme(schema).Build() + g.Expect(createNamespace(c, testcase)).Should(BeNil()) + + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testcase, + Name: testcase, + }, + Spec: appsv1.DeploymentSpec{ + Selector: selector, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: selectedLabels, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "nginx:v1", }, }, }, }, - } - Expect(mgr.GetClient().Create(context.TODO(), deploy)).Should(BeNil()) - - adapter := &OwnerAdapterImpl{ - name: testcase, - selector: selector, - selectedLabels: selectedLabels, - collisionCount: 0, - historyLimit: 0, // we at lease reserve 2 extra controller revisions - currentRevision: "test", - inUsed: false, - } + }, + } + g.Expect(c.Create(context.TODO(), deploy)).Should(BeNil()) + + adapter := &OwnerAdapterImpl{ + name: testcase, + selector: selector, + selectedLabels: selectedLabels, + collisionCount: 0, + historyLimit: 0, // we at lease reserve 2 extra controller revisions + currentRevision: "test", + inUsed: true, + } + + revisionManager := NewRevisionManager(c, schema, adapter) + revisionManager.ConstructRevisions(deploy, false) + waitingCacheUpdate(deploy.Namespace, 1) - revisionManager := NewRevisionManager(mgr.GetClient(), mgr.GetScheme(), adapter) - revisionManager.ConstructRevisions(deploy, false) - waitingCacheUpdate(deploy.Namespace, 1) + deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v2" + revisionManager.ConstructRevisions(deploy, false) + waitingCacheUpdate(deploy.Namespace, 2) - deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v2" - revisionManager.ConstructRevisions(deploy, false) - waitingCacheUpdate(deploy.Namespace, 2) + deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v3" + revisionManager.ConstructRevisions(deploy, false) + waitingCacheUpdate(deploy.Namespace, 3) - deploy.Spec.Template.Spec.Containers[0].Image = "nginx:v3" - revisionManager.ConstructRevisions(deploy, false) - waitingCacheUpdate(deploy.Namespace, 3) + revisionManager.ConstructRevisions(deploy, false) + waitingCacheUpdate(deploy.Namespace, 3) - revisionManager.ConstructRevisions(deploy, false) - waitingCacheUpdate(deploy.Namespace, 2) - }) + adapter.inUsed = false + revisionManager.ConstructRevisions(deploy, false) + waitingCacheUpdate(deploy.Namespace, 2) +} -}) +func TestRevisionCreation(t *testing.T) { + g = NewGomegaWithT(t) + testcase := "test-revision-creation" + schema := runtime.NewScheme() + corev1.AddToScheme(schema) + appsv1.AddToScheme(schema) + c = fake.NewClientBuilder().WithScheme(schema).Build() + g.Expect(createNamespace(c, testcase)).Should(BeNil()) + + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testcase, + Name: testcase, + }, + Spec: appsv1.DeploymentSpec{ + Selector: selector, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: selectedLabels, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + Image: "nginx:v1", + }, + }, + }, + }, + }, + } + g.Expect(c.Create(context.TODO(), deploy)).Should(BeNil()) + + adapter := &OwnerAdapterImpl{ + name: testcase, + selector: selector, + selectedLabels: selectedLabels, + collisionCount: 0, + historyLimit: 0, // we at lease reserve 2 extra controller revisions + currentRevision: "test", + inUsed: true, + } + + revisionManager := NewRevisionManager(c, schema, adapter) + currentRevision, _, _, _, _, err := revisionManager.ConstructRevisions(deploy, false) + g.Expect(err).Should(BeNil()) + waitingCacheUpdate(deploy.Namespace, 1) + + _, err = revisionManager.createControllerRevision(context.TODO(), deploy, currentRevision, nil) + g.Expect(err).ShouldNot(BeNil()) + + var collisionCount int32 = 0 + // if new revision conflict with existing revision and their contents are equals, then will reuse the existing one + newRevision, err := revisionManager.newRevision(deploy, currentRevision.Revision+1, &collisionCount) + g.Expect(err).Should(BeNil()) + newRevision, err = revisionManager.createControllerRevision(context.TODO(), deploy, newRevision, &collisionCount) + g.Expect(err).Should(BeNil()) + g.Expect(newRevision.Name).Should(BeEquivalentTo(currentRevision.Name)) + + // change the data of existing revision + deployClone := deploy.DeepCopy() + deployClone.Spec.Template.Labels["foo"] = "foo" + currentRevision.Data.Raw, _ = revisionManager.ownerGetter.GetPatch(deployClone) + g.Expect(c.Update(context.TODO(), currentRevision)).Should(BeNil()) + // if their contents are not equals, it should regenerate a new name + newRevision, err = revisionManager.newRevision(deploy, currentRevision.Revision+1, &collisionCount) + g.Expect(err).Should(BeNil()) + newRevision, err = revisionManager.createControllerRevision(context.TODO(), deploy, newRevision, &collisionCount) + g.Expect(err).Should(BeNil()) + g.Expect(newRevision.Name).ShouldNot(BeEquivalentTo(currentRevision.Name)) +} func waitingCacheUpdate(namespace string, expectedRevisionCount int) { - Eventually(func() error { + g.Eventually(func() error { revisionList := &appsv1.ControllerRevisionList{} if err := c.List(context.TODO(), revisionList, &client.ListOptions{Namespace: namespace}); err != nil { return err @@ -247,10 +313,6 @@ func (a OwnerAdapterImpl) GetPatch(obj metav1.Object) ([]byte, error) { return json.Marshal(obj) } -func (a OwnerAdapterImpl) GetSelectorLabels(obj metav1.Object) map[string]string { - return a.selectedLabels -} - func (a OwnerAdapterImpl) GetCurrentRevision(obj metav1.Object) string { return a.currentRevision } @@ -259,72 +321,6 @@ func (a OwnerAdapterImpl) IsInUsed(obj metav1.Object, controllerRevision string) return a.inUsed } -func TestCollaSetController(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "CollaSetController Test Suite") -} - -var _ = BeforeSuite(func() { - By("bootstrapping test environment") - - ctx, cancel = context.WithCancel(context.TODO()) - logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) - - env = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, - } - - config, err := env.Start() - Expect(err).NotTo(HaveOccurred()) - Expect(config).NotTo(BeNil()) - - mgr, err = manager.New(config, manager.Options{ - MetricsBindAddress: "0", - NewCache: inject.NewCacheWithFieldIndex, - }) - Expect(err).NotTo(HaveOccurred()) - - scheme := mgr.GetScheme() - err = appsv1.SchemeBuilder.AddToScheme(scheme) - Expect(err).NotTo(HaveOccurred()) - err = apis.AddToScheme(scheme) - Expect(err).NotTo(HaveOccurred()) - - c = mgr.GetClient() - - go func() { - err = mgr.Start(ctx) - Expect(err).NotTo(HaveOccurred()) - }() -}) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - - cancel() - - err := env.Stop() - Expect(err).NotTo(HaveOccurred()) -}) - -var _ = AfterEach(func() { - crList := &appsv1.ControllerRevisionList{} - Expect(mgr.GetClient().List(context.Background(), crList)).Should(BeNil()) - - for i := range crList.Items { - Expect(mgr.GetClient().Delete(context.TODO(), &crList.Items[i])).Should(BeNil()) - } - - nsList := &corev1.NamespaceList{} - Expect(mgr.GetClient().List(context.Background(), nsList)).Should(BeNil()) - - for i := range nsList.Items { - if strings.HasPrefix(nsList.Items[i].Name, "test-") { - mgr.GetClient().Delete(context.TODO(), &nsList.Items[i]) - } - } -}) - func createNamespace(c client.Client, namespaceName string) error { ns := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{