Merge branch 'fix/missing-namespace-fail-fast' into 'main' (d50f26cb) · Commits · OSL / MDG HypO / code / addons / ocm-scheduler

.gitlab-ci.yml

+2 −0

Changes for .gitlab-ci.yml: 2 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -99,6 +99,8 @@ tests_job:
		script:
		- go mod download
		- make test
		# make test regenerates the CRD and deepcopy code from api/: fail if the committed files are stale
		- git diff --exit-code -- config/crd/bases api \|\| { echo "Run 'make manifests generate' and commit the result"; exit 1; }
		- cp cover.out ${SONAR_GO_COVERAGE}/coverage.out
		artifacts:
		paths:

src/ocm_scheduler_operator/internal/controller/analyse.go

+1 −1

Changes for src/ocm_scheduler_operator/internal/controller/analyse.go: 1 added line, 1 removed line.
Original line number	Diff line number	Diff line
		@@ -30,7 +30,7 @@ func analyseWorkload(workload *Resource, ancillaryResources ResourceSet) (Resour
		resources := ResourceSet{}
		resourcesNotFound := AbsentResourceSet{}

		if workload.metadata.namespace != "default" {
		if workload.metadata.namespace != defaultNamespace {
		namespaceResource, err := ancillaryResources.Find(workload.metadata.namespace, "Namespace")
		if err != nil {
		return ResourceSet{}, AbsentResourceSet{}, []ServicePortPair{}, fmt.Errorf("%w", err)

src/ocm_scheduler_operator/internal/controller/hyposchedulingmanifest_controller.go

+14 −2

Changes for src/ocm_scheduler_operator/internal/controller/hyposchedulingmanifest_controller.go: 14 added lines, 2 removed lines.
Original line number	Diff line number	Diff line
		@@ -346,7 +346,7 @@ func (r *HypoSchedulingManifestReconciler) Reconcile(ctx context.Context, req ct
		return ctrl.Result{}, nil
		} else if err != nil {
		log.Error(err, "Failed to process manifests to be scheduled")
		return ctrl.Result{}, fmt.Errorf("%w", err)
		return r.failBundling(ctx, hypoSchedulingManifest, err)
		}

		// Create a bundle if necessary and set the clusterName to the targetCluster specified
		@@ -377,7 +377,7 @@ func (r *HypoSchedulingManifestReconciler) Reconcile(ctx context.Context, req ct
		return ctrl.Result{}, nil
		} else if err != nil {
		log.Error(err, "Failed to perform bundling")
		return ctrl.Result{}, fmt.Errorf("%w", err)
		return r.failBundling(ctx, hypoSchedulingManifest, err)
		}

		for _, bundle := range bundles {
		@@ -929,6 +929,18 @@ func (r *HypoSchedulingManifestReconciler) getSchedulingDecisions(bundles Bundle
		return schedulingDecisions
		}

		// failBundling sets SchedulingFailed and stops without requeuing: bundling only reads the manifests,
		// so a retry fails the same way until they change.
		func (r HypoSchedulingManifestReconciler) failBundling(ctx context.Context, hypoSchedulingManifest schedulingv1alpha1.HypoSchedulingManifest, err error) (ctrl.Result, error) {
		condition := metav1.Condition{Type: schedulingFailed, Status: metav1.ConditionTrue, Reason: "BundlingFailed", Message: utils.SentenceCase(err.Error())}
		if err := r.UpdateStatus(ctx, hypoSchedulingManifest, condition, []schedulingv1alpha1.SchedulingDecision{}); err != nil {
		log.FromContext(ctx).Error(err, updateFailure)
		return ctrl.Result{}, fmt.Errorf("%w", err)
		}

		return ctrl.Result{}, nil
		}

		// SetupWithManager sets up the controller with the Manager.
		// nolint:wrapcheck
		func (r *HypoSchedulingManifestReconciler) SetupWithManager(mgr ctrl.Manager) error {

src/ocm_scheduler_operator/internal/controller/resourceset.go

+28 −0

Changes for src/ocm_scheduler_operator/internal/controller/resourceset.go: 28 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -144,9 +144,37 @@ func bulkConvertToResourceSet(manifests []runtime.RawExtension) (ResourceSet, er
		resources.Add(&Resource{metadata: metadata, manifest: manifest})
		}
		}

		if err := resources.validateNamespacesDefined(); err != nil {
		return ResourceSet{}, err
		}

		return resources, nil
		}

		// Namespace that exists on every cluster and needs no Namespace manifest
		const defaultNamespace = "default"

		// The manifests are applied to managed clusters where nothing creates namespaces beforehand, so every
		// namespace used by a resource, other than default, must be defined by a Namespace manifest.
		// Otherwise none of the resources in that namespace can be applied.
		func (resourceSet *ResourceSet) validateNamespacesDefined() error {
		for _, resource := range *resourceSet {
		namespace := resource.metadata.namespace
		if namespace == "" \|\| namespace == defaultNamespace {
		continue
		}

		if _, err := resourceSet.Find(namespace, "Namespace"); err != nil {
		errorMessage := fmt.Sprintf("invalid manifest: %s %s uses the namespace %s, which is not defined in the manifests",
		strings.ToLower(resource.metadata.groupVersionKind.Kind), resource.metadata.name, namespace)
		return &MisconfiguredManifestError{errorMessage}
		}
		}

		return nil
		}

		func (m ManifestMetadata) isValid() error {
		if !m.isSupportedResource() {
		errorMessage := fmt.Sprintf("unsupported manifest: the cluster admin is responsible for managing %s resources", strings.ToLower(m.groupVersionKind.Kind))

src/ocm_scheduler_operator/internal/controller/resourceset_test.go

0 → 100644

+97 −0

Changes for src/ocm_scheduler_operator/internal/controller/resourceset_test.go: 97 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		package controller

		import (
		"fmt"

		. "github.com/onsi/ginkgo/v2"
		. "github.com/onsi/gomega"
		metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
		"k8s.io/apimachinery/pkg/runtime"

		schedulingv1alpha1 "github.com/rh-waterford-et/hypo-scheduler-operator/api/v1alpha1"
		)

		var _ = Describe("Namespaces used by the manifests", func() {
		deployment := func(namespace string) runtime.RawExtension {
		return runtime.RawExtension{Raw: []byte(fmt.Sprintf(
		`{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"web","namespace":%q},`+
		`"spec":{"selector":{"matchLabels":{"app":"web"}},"template":{"metadata":{"labels":{"app":"web"}},`+
		`"spec":{"containers":[{"name":"web","image":"nginx"}]}}}}`, namespace))}
		}
		namespace := func(name string) runtime.RawExtension {
		return runtime.RawExtension{Raw: []byte(fmt.Sprintf(`{"apiVersion":"v1","kind":"Namespace","metadata":{"name":%q}}`, name))}
		}
		clusterRole := runtime.RawExtension{Raw: []byte(`{"apiVersion":"rbac.authorization.k8s.io/v1","kind":"ClusterRole","metadata":{"name":"reader"}}`)}

		expectMisconfigured := func(err error) {
		var misconfiguredManifestErr *MisconfiguredManifestError
		Expect(err).To(HaveOccurred())
		Expect(err).To(BeAssignableToTypeOf(misconfiguredManifestErr))
		Expect(err.Error()).To(Equal("invalid manifest: deployment web uses the namespace foo, which is not defined in the manifests"))
		}

		manifest := func(globalAnnotations []string, workloadAnnotations []schedulingv1alpha1.WorkloadAnnotation, manifests ...runtime.RawExtension) *schedulingv1alpha1.HypoSchedulingManifest {
		return &schedulingv1alpha1.HypoSchedulingManifest{
		ObjectMeta: metav1.ObjectMeta{Name: "test"},
		Spec: schedulingv1alpha1.HypoSchedulingManifestSpec{
		GlobalAnnotations: globalAnnotations,
		WorkloadAnnotations: workloadAnnotations,
		Manifests: manifests,
		},
		}
		}

		Context("bulkConvertToResourceSet", func() {
		It("rejects a resource whose namespace has no Namespace manifest", func() {
		_, err := bulkConvertToResourceSet([]runtime.RawExtension{deployment("foo")})
		expectMisconfigured(err)
		})

		It("accepts a resource whose namespace has a Namespace manifest", func() {
		resources, err := bulkConvertToResourceSet([]runtime.RawExtension{deployment("foo"), namespace("foo")})
		Expect(err).NotTo(HaveOccurred())
		Expect(resources).To(HaveLen(2))
		})

		It("accepts the default namespace without a Namespace manifest", func() {
		_, err := bulkConvertToResourceSet([]runtime.RawExtension{deployment("default")})
		Expect(err).NotTo(HaveOccurred())
		})

		It("accepts cluster-scoped resources", func() {
		_, err := bulkConvertToResourceSet([]runtime.RawExtension{clusterRole, namespace("foo")})
		Expect(err).NotTo(HaveOccurred())
		})
		})

		Context("buildAndUpdateBundles", func() {
		It("fails fast on the global bundle path", func() {
		_, err := buildAndUpdateBundles(BundleList{}, manifest([]string{"hypo.filter.location=greece"}, nil, deployment("foo")))
		expectMisconfigured(err)
		})

		It("fails fast on the default bundle path", func() {
		_, err := buildAndUpdateBundles(BundleList{}, manifest(nil, nil, deployment("foo")))
		expectMisconfigured(err)
		})

		// Before the fix this path returned a ResourceNotFoundError, which left the instance in
		// SchedulingInProgress while it was requeued forever
		It("fails fast on the per-workload path", func() {
		workloadAnnotations := []schedulingv1alpha1.WorkloadAnnotation{{Name: "web", Annotations: []string{"hypo.filter.location=greece"}}}
		_, err := buildAndUpdateBundles(BundleList{}, manifest(nil, workloadAnnotations, deployment("foo")))
		expectMisconfigured(err)
		})

		It("bundles the workload with its Namespace on the per-workload path", func() {
		workloadAnnotations := []schedulingv1alpha1.WorkloadAnnotation{{Name: "web", Annotations: []string{"hypo.filter.location=greece"}}}
		bundles, err := buildAndUpdateBundles(BundleList{}, manifest(nil, workloadAnnotations, deployment("foo"), namespace("foo")))
		Expect(err).NotTo(HaveOccurred())

		bundle := bundles.getBundle("web")
		Expect(bundle).NotTo(BeNil())
		_, err = bundle.resources.Find("foo", "Namespace")
		Expect(err).NotTo(HaveOccurred())
		})
		})
		})