diff --git a/pkg/controllers/deliverable_reconciler_test.go b/pkg/controllers/deliverable_reconciler_test.go index 724ba81e6..8ef6dc4ae 100644 --- a/pkg/controllers/deliverable_reconciler_test.go +++ b/pkg/controllers/deliverable_reconciler_test.go @@ -271,7 +271,7 @@ var _ = Describe("DeliverableReconciler", func() { Name: "my-image-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false) + }, 0, nil, false) resourceStatuses.Add( &v1alpha1.RealizedResource{ Name: "resource2", @@ -287,7 +287,7 @@ var _ = Describe("DeliverableReconciler", func() { Name: "my-config-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false) + }, 1, nil, false) rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error { statusesVal := reflect.ValueOf(statuses) @@ -632,7 +632,7 @@ var _ = Describe("DeliverableReconciler", func() { Name: "my-image-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false, + }, 0, nil, false, ) resourceStatuses.Add( &v1alpha1.RealizedResource{ @@ -643,7 +643,7 @@ var _ = Describe("DeliverableReconciler", func() { Name: "my-config-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false, + }, 1, nil, false, ) rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error { @@ -1592,7 +1592,7 @@ var _ = Describe("DeliverableReconciler", func() { Kind: "some-template-kind", Name: "some-template-name", }, - }, nil, false, + }, 0, nil, false, ) rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error { statusesVal := reflect.ValueOf(statuses) diff --git a/pkg/controllers/workload_reconciler_test.go b/pkg/controllers/workload_reconciler_test.go index 42de12050..56895cfeb 100644 --- a/pkg/controllers/workload_reconciler_test.go +++ b/pkg/controllers/workload_reconciler_test.go @@ -271,7 +271,7 @@ var _ = Describe("WorkloadReconciler", func() { Name: "my-image-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false, + }, 0, nil, false, ) resourceStatuses.Add( &v1alpha1.RealizedResource{ @@ -288,7 +288,7 @@ var _ = Describe("WorkloadReconciler", func() { Name: "my-config-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false, + }, 1, nil, false, ) rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error { @@ -632,7 +632,7 @@ var _ = Describe("WorkloadReconciler", func() { Name: "my-image-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false, + }, 0, nil, false, ) resourceStatuses.Add( &v1alpha1.RealizedResource{ @@ -643,7 +643,7 @@ var _ = Describe("WorkloadReconciler", func() { Name: "my-config-template", APIVersion: "carto.run/v1alpha1", }, - }, nil, false, + }, 1, nil, false, ) rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error { @@ -1381,7 +1381,7 @@ var _ = Describe("WorkloadReconciler", func() { Kind: "some-template-kind", Name: "some-template-name", }, - }, nil, false, + }, 0, nil, false, ) rlzr.RealizeStub = func(ctx context.Context, resourceRealizer realizer.ResourceRealizer, deliveryName string, resources []realizer.OwnerResource, statuses statuses.ResourceStatuses) error { statusesVal := reflect.ValueOf(statuses) diff --git a/pkg/realizer/realizer.go b/pkg/realizer/realizer.go index f14ebbbd9..3cb802d13 100644 --- a/pkg/realizer/realizer.go +++ b/pkg/realizer/realizer.go @@ -111,7 +111,7 @@ func (r *realizer) Realize(ctx context.Context, resourceRealizer ResourceRealize outs := NewOutputs() var firstError error - for _, resource := range ownerResources { + for resourceIndex, resource := range ownerResources { log = log.WithValues("resource", resource.Name) ctx = logr.NewContext(ctx, log) template, stampedObject, out, isPassThrough, templateName, err := resourceRealizer.Do(ctx, resource, blueprintName, outs, r.mapper) @@ -168,7 +168,7 @@ func (r *realizer) Realize(ctx context.Context, resourceRealizer ResourceRealize } } - resourceStatuses.Add(realizedResource, err, isPassThrough, additionalConditions...) + resourceStatuses.Add(realizedResource, resourceIndex, err, isPassThrough, additionalConditions...) if slices.Contains(resourceStatuses.ChangedConditionTypes(realizedResource.Name), v1alpha1.ResourceHealthy) { newStatus := metav1.ConditionUnknown diff --git a/pkg/realizer/realizer_test.go b/pkg/realizer/realizer_test.go index 760d120ad..aea3f9a05 100644 --- a/pkg/realizer/realizer_test.go +++ b/pkg/realizer/realizer_test.go @@ -518,6 +518,7 @@ var _ = Describe("Realize", func() { previousResources []v1alpha1.ResourceStatus previousTime metav1.Time supplyChain *v1alpha1.ClusterSupplyChain + resource2 v1alpha1.SupplyChainResource ) BeforeEach(func() { previousTime = metav1.NewTime(time.Now()) @@ -593,7 +594,7 @@ var _ = Describe("Realize", func() { resource1 := v1alpha1.SupplyChainResource{ Name: "resource1", } - resource2 := v1alpha1.SupplyChainResource{ + resource2 = v1alpha1.SupplyChainResource{ Name: "resource2", Sources: []v1alpha1.ResourceReference{ { @@ -691,6 +692,10 @@ var _ = Describe("Realize", func() { var resource2Status v1alpha1.ResourceStatus var resource3Status v1alpha1.ResourceStatus + for i := range currentStatuses { + Expect(currentStatuses[i].Name).To(Equal(fmt.Sprintf("resource%d", i+1))) + } + for i := range currentStatuses { switch currentStatuses[i].Name { case "resource1": @@ -743,6 +748,44 @@ var _ = Describe("Realize", func() { Expect(resourceObj).To(Equal(stampedObj1)) }) + Context("the supply chain has changed to have fewer resources", func() { + var currentStatuses []v1alpha1.ResourceStatus + BeforeEach(func() { + supplyChain = &v1alpha1.ClusterSupplyChain{ + ObjectMeta: metav1.ObjectMeta{Name: "greatest-supply-chain"}, + Spec: v1alpha1.SupplyChainSpec{ + Resources: []v1alpha1.SupplyChainResource{resource2}, + }, + } + + template2 := &v1alpha1.ClusterImageTemplate{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-image-template", + }, + } + reader2, err := templates.NewReaderFromAPI(template2) + Expect(err).NotTo(HaveOccurred()) + + resourceRealizer.DoReturnsOnCall(0, reader2, &unstructured.Unstructured{}, nil, false, resource2.Name, nil) + + oldOutput := &templates.Output{ + Image: "whatever", + } + resourceRealizer.DoReturnsOnCall(0, reader2, &unstructured.Unstructured{}, oldOutput, false, "", nil) + + resourceStatuses := statuses.NewResourceStatuses(previousResources, conditions.AddConditionForResourceSubmittedWorkload) + err = rlzr.Realize(ctx, resourceRealizer, supplyChain.Name, realizer.MakeSupplychainOwnerResources(supplyChain), resourceStatuses) + Expect(err).ToNot(HaveOccurred()) + + currentStatuses = resourceStatuses.GetCurrent() + }) + It("only creates 1 resource status in currentStatuses", func() { + Expect(currentStatuses[0].Name).To(Equal("resource2")) + Expect(len(currentStatuses)).To(Equal(1)) + }) + }) + Context("there is an error realizing resource 1 and resource 2", func() { BeforeEach(func() { resourceRealizer.DoReturnsOnCall(0, nil, nil, nil, false, "", errors.New("im in a bad state")) diff --git a/pkg/realizer/statuses/resource_statuses.go b/pkg/realizer/statuses/resource_statuses.go index 961e64779..5cfe471c5 100644 --- a/pkg/realizer/statuses/resource_statuses.go +++ b/pkg/realizer/statuses/resource_statuses.go @@ -27,7 +27,7 @@ import ( type ResourceStatuses interface { ChangedConditionTypes(realizedResourceName string) []string GetPreviousResourceStatus(realizedResourceName string) *v1alpha1.ResourceStatus - Add(status *v1alpha1.RealizedResource, err error, isPassThrough bool, furtherConditions ...metav1.Condition) + Add(status *v1alpha1.RealizedResource, resourceIndex int, err error, isPassThrough bool, furtherConditions ...metav1.Condition) GetCurrent() ResourceStatusList IsChanged() bool } @@ -114,7 +114,7 @@ func (r *resourceStatuses) GetPreviousResourceStatus(realizedResourceName string return nil } -func (r *resourceStatuses) Add(realizedResource *v1alpha1.RealizedResource, err error, isPassThrough bool, furtherConditions ...metav1.Condition) { +func (r *resourceStatuses) Add(realizedResource *v1alpha1.RealizedResource, resourceIndex int, err error, isPassThrough bool, furtherConditions ...metav1.Condition) { name := realizedResource.Name var existingStatus *resourceStatus @@ -129,7 +129,9 @@ func (r *resourceStatuses) Add(realizedResource *v1alpha1.RealizedResource, err existingStatus = &resourceStatus{ name: name, } - r.statuses = append(r.statuses, existingStatus) + r.statuses = append(r.statuses, &resourceStatus{}) + copy(r.statuses[resourceIndex+1:], r.statuses[resourceIndex:len(r.statuses)-1]) + r.statuses[resourceIndex] = existingStatus } existingStatus.current = &v1alpha1.ResourceStatus{ diff --git a/pkg/realizer/statuses/resource_statuses_test.go b/pkg/realizer/statuses/resource_statuses_test.go index 561d782ee..90f83077d 100644 --- a/pkg/realizer/statuses/resource_statuses_test.go +++ b/pkg/realizer/statuses/resource_statuses_test.go @@ -77,7 +77,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false) + }, 0, nil, false) }) It("the resourceStatuses reports IsChanged is false", func() { @@ -97,7 +97,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false) + }, 1, nil, false) Expect(resourceStatuses.IsChanged()).To(BeTrue()) }) @@ -108,7 +108,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false) + }, 1, nil, false) Expect(resourceStatuses.ChangedConditionTypes("resource2")).To(ConsistOf(v1alpha1.ResourceSubmitted, v1alpha1.ResourceReady)) }) @@ -119,7 +119,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false, metav1.Condition{ + }, 1, nil, false, metav1.Condition{ Type: v1alpha1.ResourceHealthy, Status: "Unknown", }) @@ -133,7 +133,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false, metav1.Condition{ + }, 1, nil, false, metav1.Condition{ Type: v1alpha1.ResourceHealthy, Status: "False", }) @@ -147,7 +147,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false, metav1.Condition{ + }, 1, nil, false, metav1.Condition{ Type: v1alpha1.ResourceHealthy, Status: "True", }) @@ -168,7 +168,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false) + }, 0, nil, false) Expect(resourceStatuses.IsChanged()).To(BeTrue()) }) }) @@ -181,7 +181,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, errors.New("has an error"), false) + }, 0, errors.New("has an error"), false) Expect(resourceStatuses.IsChanged()).To(BeTrue()) }) @@ -192,7 +192,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, errors.New("has an error"), false) + }, 0, errors.New("has an error"), false) Expect(resourceStatuses.ChangedConditionTypes("resource1")).To(ConsistOf("Ready", "ResourceSubmitted")) }) }) @@ -217,7 +217,7 @@ var _ = Describe("ResourceStatuses", func() { TemplateRef: nil, Inputs: nil, Outputs: nil, - }, nil, false) + }, 0, nil, false) Expect(resourceStatuses.IsChanged()).To(BeTrue()) }) }) diff --git a/tests/integration/supplychain/workload_reconciler_test.go b/tests/integration/supplychain/workload_reconciler_test.go index e35c910b5..a4636ab78 100644 --- a/tests/integration/supplychain/workload_reconciler_test.go +++ b/tests/integration/supplychain/workload_reconciler_test.go @@ -313,6 +313,12 @@ var _ = Describe("WorkloadReconciler", func() { "Status": Equal(metav1.ConditionStatus("Unknown")), }), )) + obj := &v1alpha1.Workload{} + err = c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + Expect(len(obj.Status.Resources)).To(Equal(1)) + Expect(obj.Status.Resources[0].RealizedResource.Name).To(Equal("my-first-resource")) }) Context("a stamped object has changed", func() { @@ -379,6 +385,192 @@ var _ = Describe("WorkloadReconciler", func() { }))) }) }) + + Context("the workload is changed to a new supply chain with resources in a new order", func() { + BeforeEach(func() { + simpleTemplateYaml := utils.HereYaml(` + --- + apiVersion: carto.run/v1alpha1 + kind: ClusterTemplate + metadata: + name: simple-template + spec: + template: + apiVersion: test.run/v1alpha1 + kind: TestObj + metadata: + name: another-resource-$(params.suffix)$ + spec: + baz: "qux" + `) + template := utils.CreateObjectOnClusterFromYamlDefinition(ctx, c, simpleTemplateYaml) + cleanups = append(cleanups, template) + + supplyChainYaml := utils.HereYaml(` + --- + apiVersion: carto.run/v1alpha1 + kind: ClusterSupplyChain + metadata: + name: new-supply-chain + spec: + selector: + "another-key": "another-value" + resources: + - name: my-zeroth-resource + templateRef: + kind: ClusterTemplate + name: simple-template + params: + - name: suffix + default: 0 + - name: my-first-resource + templateRef: + kind: ClusterConfigTemplate + name: my-config-template + - name: another-resource + templateRef: + kind: ClusterTemplate + name: simple-template + params: + - name: suffix + default: another + `) + supplyChain := utils.CreateObjectOnClusterFromYamlDefinition(ctx, c, supplyChainYaml) + cleanups = append(cleanups, supplyChain) + + workload := &v1alpha1.Workload{} + err := c.Get(context.Background(), client.ObjectKey{Name: "workload-joe", Namespace: testNS}, workload) + Expect(err).NotTo(HaveOccurred()) + + workload.ObjectMeta.Labels = map[string]string{ + "another-key": "another-value", + } + + err = c.Update(context.Background(), workload) + Expect(err).NotTo(HaveOccurred()) + + Eventually(func() int { + obj := &v1alpha1.Workload{} + err := c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + return len(obj.Status.Resources) + }).Should(Equal(3)) + }) + + It("it updates to use new supply-chain", func() { + obj := &v1alpha1.Workload{} + err := c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + Expect(obj.Status.Resources[0].RealizedResource.Name).To(Equal("my-zeroth-resource")) + Expect(obj.Status.Resources[1].RealizedResource.Name).To(Equal("my-first-resource")) + Expect(obj.Status.Resources[2].RealizedResource.Name).To(Equal("another-resource")) + }) + + Context("supply chain deletes earlier resources", func() { + BeforeEach(func() { + supplyChainYaml := utils.HereYaml(` + --- + apiVersion: carto.run/v1alpha1 + kind: ClusterSupplyChain + metadata: + name: smaller-supply-chain + spec: + selector: + "what-kept": "later" + resources: + - name: another-resource + templateRef: + kind: ClusterTemplate + name: simple-template + params: + - name: suffix + default: another + `) + supplyChain := utils.CreateObjectOnClusterFromYamlDefinition(ctx, c, supplyChainYaml) + cleanups = append(cleanups, supplyChain) + + workload := &v1alpha1.Workload{} + err := c.Get(context.Background(), client.ObjectKey{Name: "workload-joe", Namespace: testNS}, workload) + Expect(err).NotTo(HaveOccurred()) + + workload.ObjectMeta.Labels = map[string]string{ + "what-kept": "later", + } + + err = c.Update(context.Background(), workload) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns only the existing resource in the workload status", func() { + Eventually(func() int { + obj := &v1alpha1.Workload{} + err := c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + return len(obj.Status.Resources) + }).Should(Equal(1)) + + obj := &v1alpha1.Workload{} + err := c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + Expect(obj.Status.Resources[0].RealizedResource.Name).To(Equal("another-resource")) + }) + }) + Context("supply chain deletes later resources", func() { + BeforeEach(func() { + supplyChainYaml := utils.HereYaml(` + --- + apiVersion: carto.run/v1alpha1 + kind: ClusterSupplyChain + metadata: + name: smaller-supply-chain + spec: + selector: + "what-kept": "earlier" + resources: + - name: my-zeroth-resource + templateRef: + kind: ClusterTemplate + name: simple-template + params: + - name: suffix + default: 0 + `) + supplyChain := utils.CreateObjectOnClusterFromYamlDefinition(ctx, c, supplyChainYaml) + cleanups = append(cleanups, supplyChain) + + workload := &v1alpha1.Workload{} + err := c.Get(context.Background(), client.ObjectKey{Name: "workload-joe", Namespace: testNS}, workload) + Expect(err).NotTo(HaveOccurred()) + + workload.ObjectMeta.Labels = map[string]string{ + "what-kept": "earlier", + } + + err = c.Update(context.Background(), workload) + Expect(err).NotTo(HaveOccurred()) + }) + + It("returns only the existing resource in the workload status", func() { + Eventually(func() int { + obj := &v1alpha1.Workload{} + err := c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + return len(obj.Status.Resources) + }).Should(Equal(1)) + + obj := &v1alpha1.Workload{} + err := c.Get(ctx, client.ObjectKey{Name: "workload-joe", Namespace: testNS}, obj) + Expect(err).NotTo(HaveOccurred()) + + Expect(obj.Status.Resources[0].RealizedResource.Name).To(Equal("my-zeroth-resource")) + }) + }) + }) }) Context("supply chain with immutable template", func() {