fix(feature): ensures all new resources are created in one reconcile (o…

…pendatahub-io#1159)

Manifests used by Feature to create cluster resources can be either
defined as a single resource per file or grouped together and separated
using the `---` delimiter. Even though the former approach is currently
preferred across the codebase, the latter is used to create Envoy filters
for the KServe setup.

When applying resources not yet present in the cluster, the original
implementation returns after creating the first resource, leaving the rest
unprocessed. They are applied in the next reconcile loop, though again
on a one-per-reconcile event basis. This is not a bug per se, as it will
eventually result in creating all resources; however, it is suboptimal.

This change ensures all resources defined in a single YAML manifest,
separated using `---`, are processed in a single go.

pkg/feature/resource/operations.go

@@ -31,15 +31,16 @@ func Apply(ctx context.Context, cli client.Client, objects []*unstructured.Unstr
 			return fmt.Errorf("failed to get resource %s/%s: %w", namespace, name, errGet)
 		}
 
+		justCreated := false
 		if k8serr.IsNotFound(errGet) {
 			if errCreate := cli.Create(ctx, target); client.IgnoreAlreadyExists(errCreate) != nil {
 				return fmt.Errorf("failed to create source %s/%s: %w", namespace, name, errCreate)
 			}
 
-			return nil
+			justCreated = true
 		}
 
-		if shouldReconcile(source) {
+		if !justCreated && shouldReconcile(source) {
 			if errUpdate := patchUsingApplyStrategy(ctx, cli, source, target); errUpdate != nil {
 				return fmt.Errorf("failed to reconcile resource %s/%s: %w", namespace, name, errUpdate)
 			}

tests/integration/features/fixtures/templates/namespaces.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: embedded-test-ns-1
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: embedded-test-ns-2
+

tests/integration/features/manifests_int_test.go

@@ -46,11 +46,11 @@ var _ = Describe("Applying resources", func() {
 	It("should be able to process an embedded YAML file", func(ctx context.Context) {
 		// given
 		featuresHandler := feature.ClusterFeaturesHandler(dsci, func(registry feature.FeaturesRegistry) error {
-			errNsCreate := registry.Add(feature.Define("create-namespace").
+			errNsCreate := registry.Add(feature.Define("create-namespaces").
 				UsingConfig(envTest.Config).
 				Manifests(
 					manifest.Location(fixtures.TestEmbeddedFiles).
-						Include(path.Join(fixtures.BaseDir, "namespace.yaml")),
+						Include(path.Join(fixtures.BaseDir, "namespaces.yaml")),
 				),
 			)
 
@@ -63,10 +63,15 @@ var _ = Describe("Applying resources", func() {
 		Expect(featuresHandler.Apply(ctx)).To(Succeed())
 
 		// then
-		embeddedNs, err := fixtures.GetNamespace(ctx, envTestClient, "embedded-test-ns")
-		defer objectCleaner.DeleteAll(ctx, embeddedNs)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(embeddedNs.Name).To(Equal("embedded-test-ns"))
+		embeddedNs1, errNS1 := fixtures.GetNamespace(ctx, envTestClient, "embedded-test-ns-1")
+		embeddedNs2, errNS2 := fixtures.GetNamespace(ctx, envTestClient, "embedded-test-ns-2")
+		defer objectCleaner.DeleteAll(ctx, embeddedNs1, embeddedNs2)
+
+		Expect(errNS1).ToNot(HaveOccurred())
+		Expect(errNS2).ToNot(HaveOccurred())
+
+		Expect(embeddedNs1.Name).To(Equal("embedded-test-ns-1"))
+		Expect(embeddedNs2.Name).To(Equal("embedded-test-ns-2"))
 	})
 
 	It("should be able to process an embedded template file", func(ctx context.Context) {