From 2f31a1c67e6a3057ee7f784c0878d735eaea3bf6 Mon Sep 17 00:00:00 2001 From: Gianluca Mardente Date: Mon, 17 Aug 2026 19:35:13 +0200 Subject: [PATCH] chore: Enable validate health tests with Jobs in pull mode --- controllers/handlers_utils.go | 8 +- lib/clusterops/jobhealthcheck_default.go | 12 +- lib/clusterops/validate_health.go | 113 +++++++++++++----- lib/clusterops/validate_health_test.go | 142 +++++++++++++++++++++++ test/fv/job_health_check_test.go | 4 +- test/pullmode-sveltosapplier.yaml | 2 +- 6 files changed, 236 insertions(+), 45 deletions(-) diff --git a/controllers/handlers_utils.go b/controllers/handlers_utils.go index 081b4c83..efa060be 100644 --- a/controllers/handlers_utils.go +++ b/controllers/handlers_utils.go @@ -1676,12 +1676,12 @@ func prepareSetters(ctx context.Context, clusterSummary *configv1beta1.ClusterSu if includeDeployChecks { preDeployCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(), - clusterSummary, getSveltosNamespace(), clusterSummary.Spec.ClusterProfileSpec.PreDeployChecks, logger) + clusterSummary, clusterSummary.Spec.ClusterProfileSpec.PreDeployChecks, logger) if err != nil { return nil, err } validateHealthJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(), - clusterSummary, getSveltosNamespace(), clusterSummary.Spec.ClusterProfileSpec.ValidateHealths, logger) + clusterSummary, clusterSummary.Spec.ClusterProfileSpec.ValidateHealths, logger) if err != nil { return nil, err } @@ -1695,12 +1695,12 @@ func prepareSetters(ctx context.Context, clusterSummary *configv1beta1.ClusterSu if includeDeleteChecks { preDeleteCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(), - clusterSummary, getSveltosNamespace(), clusterSummary.Spec.ClusterProfileSpec.PreDeleteChecks, logger) + clusterSummary, clusterSummary.Spec.ClusterProfileSpec.PreDeleteChecks, logger) if err != nil { return nil, err } postDeleteCheckJobs, err := clusterops.ResolveJobChecksForPullMode(ctx, getManagementClusterDirectClient(), - clusterSummary, getSveltosNamespace(), clusterSummary.Spec.ClusterProfileSpec.PostDeleteChecks, logger) + clusterSummary, clusterSummary.Spec.ClusterProfileSpec.PostDeleteChecks, logger) if err != nil { return nil, err } diff --git a/lib/clusterops/jobhealthcheck_default.go b/lib/clusterops/jobhealthcheck_default.go index 134d8efb..a46c7f9b 100644 --- a/lib/clusterops/jobhealthcheck_default.go +++ b/lib/clusterops/jobhealthcheck_default.go @@ -21,24 +21,18 @@ import ( "fmt" "github.com/go-logr/logr" - batchv1 "k8s.io/api/batch/v1" libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" ) // Default JobCheck implementation: reports the feature is unavailable. A Sveltos Enterprise -// build overrides this via SetJobHealthCheckValidator/SetJobHealthCheckResolver before -// starting the manager. +// build overrides this via SetJobHealthCheckValidator before starting the manager. Fetching a +// JobCheck's Job manifest (FetchJobManifest, in validate_health.go) has no equivalent stub - +// it carries no license requirement, so it needs no Sveltos Enterprise override. func init() { validateJobHealthCheck = func(_ context.Context, _ JobHealthCheckDeps, check *libsveltosv1beta1.ValidateHealth, _ logr.Logger) error { return fmt.Errorf("JobCheck (%s) requires a Sveltos Enterprise build", check.Name) } - - resolveJobHealthCheck = func(_ context.Context, _ JobHealthCheckDeps, - check *libsveltosv1beta1.ValidateHealth, _ logr.Logger) (*batchv1.Job, error) { - - return nil, fmt.Errorf("JobCheck (%s) requires a Sveltos Enterprise build", check.Name) - } } diff --git a/lib/clusterops/validate_health.go b/lib/clusterops/validate_health.go index b7e47154..d90f2621 100644 --- a/lib/clusterops/validate_health.go +++ b/lib/clusterops/validate_health.go @@ -30,10 +30,12 @@ import ( "github.com/go-logr/logr" lua "github.com/yuin/gopher-lua" batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" @@ -46,6 +48,7 @@ import ( "github.com/projectsveltos/libsveltos/lib/cel" logs "github.com/projectsveltos/libsveltos/lib/logsettings" sveltoslua "github.com/projectsveltos/libsveltos/lib/lua" + libsveltostemplate "github.com/projectsveltos/libsveltos/lib/template" ) const ( @@ -104,34 +107,91 @@ func SetJobHealthCheckValidator(fn func(ctx context.Context, deps JobHealthCheck validateJobHealthCheck = fn } -// resolveJobHealthCheck fetches and resolves (Cluster-field templating applied) the Job -// manifest referenced by check.JobCheck.JobRef, without running it. Used only when staging a -// pull-mode ConfigurationGroup: sveltos-applier cannot verify a Sveltos Enterprise license or -// fetch JobRef itself, so addon-controller resolves the Job once, up front, here, and stages -// the result for sveltos-applier to run unattended. Same default-stub/SetX-override pattern as -// validateJobHealthCheck. -var ( - resolveJobHealthCheck func(ctx context.Context, deps JobHealthCheckDeps, - check *libsveltosv1beta1.ValidateHealth, logger logr.Logger) (*batchv1.Job, error) -) +// FetchJobManifest resolves jobRef (Cluster-field templating on namespace/name, same as any +// other referenced resource) and decodes the referenced ConfigMap/Secret's content into a Job. +// It fails if the referenced resource does not contain exactly one valid Job manifest. +// +// This has no proprietary logic of its own - it's a plain read plus the same +// libsveltos/lib/template resolution every other PolicyRef-style reference already uses - so, +// unlike running a JobCheck (which does require a Sveltos Enterprise license), fetching the +// manifest does not. Used both by ResolveJobChecksForPullMode below (addon-controller resolving +// a Job to stage for sveltos-applier to run in pull mode - itself gated by the pull-mode +// cluster-count license check, not a JobHealthCheck-specific one) and by the Sveltos Enterprise +// JobCheck Validator (push mode, after its own separate license check). +func FetchJobManifest(ctx context.Context, mgmtClient client.Client, clusterNamespace, clusterName string, + clusterType libsveltosv1beta1.ClusterType, jobRef *libsveltosv1beta1.PolicyRef, logger logr.Logger) (*batchv1.Job, error) { + + namespace, err := libsveltostemplate.GetReferenceResourceNamespace(ctx, mgmtClient, + clusterNamespace, clusterName, jobRef.Namespace, clusterType) + if err != nil { + return nil, fmt.Errorf("instantiating jobRef namespace: %w", err) + } + + name, err := libsveltostemplate.GetReferenceResourceName(ctx, mgmtClient, + clusterNamespace, clusterName, jobRef.Name, clusterType) + if err != nil { + return nil, fmt.Errorf("instantiating jobRef name: %w", err) + } + + nsName := types.NamespacedName{Namespace: namespace, Name: name} -// SetJobHealthCheckResolver overrides the JobCheck resolution implementation. Called by a -// Sveltos Enterprise build's composition root before starting the manager, alongside -// SetJobHealthCheckValidator; this package never imports anything private itself. -func SetJobHealthCheckResolver(fn func(ctx context.Context, deps JobHealthCheckDeps, - check *libsveltosv1beta1.ValidateHealth, logger logr.Logger) (*batchv1.Job, error)) { + var data map[string][]byte + switch jobRef.Kind { + case string(libsveltosv1beta1.ConfigMapReferencedResourceKind): + configMap := &corev1.ConfigMap{} + if err := mgmtClient.Get(ctx, nsName, configMap); err != nil { + return nil, fmt.Errorf("getting ConfigMap %s: %w", nsName, err) + } + data = make(map[string][]byte, len(configMap.Data)) + for k, v := range configMap.Data { + data[k] = []byte(v) + } + case string(libsveltosv1beta1.SecretReferencedResourceKind): + secret := &corev1.Secret{} + if err := mgmtClient.Get(ctx, nsName, secret); err != nil { + return nil, fmt.Errorf("getting Secret %s: %w", nsName, err) + } + data = secret.Data + default: + return nil, fmt.Errorf("jobRef.kind %q is neither ConfigMap nor Secret", jobRef.Kind) + } + + if len(data) != 1 { + return nil, fmt.Errorf("%s %s must contain exactly one Job manifest, found %d", + jobRef.Kind, nsName, len(data)) + } - resolveJobHealthCheck = fn + var content []byte + for k := range data { + content = data[k] + } + + job := &batchv1.Job{} + if err := yaml.Unmarshal(content, job); err != nil { + return nil, fmt.Errorf("%s %s does not contain a valid Job manifest: %w", jobRef.Kind, nsName, err) + } + + if job.Kind != "" && job.Kind != "Job" { + return nil, fmt.Errorf("%s %s does not contain a Job manifest (kind: %q)", jobRef.Kind, nsName, job.Kind) + } + + if len(job.Spec.Template.Spec.Containers) == 0 { + return nil, fmt.Errorf("%s %s does not contain a valid Job manifest (no containers)", jobRef.Kind, nsName) + } + + logger.V(logs.LogDebug).Info("fetched Job manifest", "job", client.ObjectKeyFromObject(job)) + + return job, nil } // ResolveJobChecksForPullMode resolves every JobCheck entry in checks (fetching JobRef's -// ConfigMap/Secret and applying Cluster-field templating, same as the push-mode path) and -// returns the resolved manifests as YAML, keyed by "/" - ready to -// stage onto a ConfigurationGroup's PreDeployCheckJobs/ValidateHealthJobs/PreDeleteCheckJobs/ -// PostDeleteCheckJobs field. Returns an empty, non-nil map when checks has no JobCheck entries. -// Fails the same way a push-mode JobCheck failure would (license, fetch, or templating error). +// ConfigMap/Secret and applying Cluster-field templating, via FetchJobManifest - no license +// check: see FetchJobManifest's comment) and returns the resolved manifests as YAML, keyed by +// "/" - ready to stage onto a ConfigurationGroup's +// PreDeployCheckJobs/ValidateHealthJobs/PreDeleteCheckJobs/PostDeleteCheckJobs field. Returns +// an empty, non-nil map when checks has no JobCheck entries. func ResolveJobChecksForPullMode(ctx context.Context, mgmtClient client.Client, - clusterSummary *configv1beta1.ClusterSummary, sveltosNamespace string, + clusterSummary *configv1beta1.ClusterSummary, checks []libsveltosv1beta1.ValidateHealth, logger logr.Logger) (map[string]string, error) { jobs := make(map[string]string) @@ -142,13 +202,8 @@ func ResolveJobChecksForPullMode(ctx context.Context, mgmtClient client.Client, continue } - deps := JobHealthCheckDeps{ - MgmtClient: mgmtClient, - ClusterSummary: clusterSummary, - SveltosNamespace: sveltosNamespace, - } - - job, err := resolveJobHealthCheck(ctx, deps, check, logger) + job, err := FetchJobManifest(ctx, mgmtClient, clusterSummary.Spec.ClusterNamespace, + clusterSummary.Spec.ClusterName, clusterSummary.Spec.ClusterType, &check.JobCheck.JobRef, logger) if err != nil { return nil, fmt.Errorf("failed to resolve JobCheck %q: %w", check.Name, err) } diff --git a/lib/clusterops/validate_health_test.go b/lib/clusterops/validate_health_test.go index 76658957..d297c33c 100644 --- a/lib/clusterops/validate_health_test.go +++ b/lib/clusterops/validate_health_test.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/klog/v2/textlogger" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/projectsveltos/addon-controller/lib/clusterops" libsveltosv1beta1 "github.com/projectsveltos/libsveltos/api/v1beta1" @@ -40,8 +41,23 @@ const ( luaFileName = "lua_policy.lua" validFileName = "valid_resource.yaml" invalidFileName = "invalid_resource.yaml" + + jobManifestDataKey = "job.yaml" ) +const jobManifest = `apiVersion: batch/v1 +kind: Job +metadata: + name: probe +spec: + template: + spec: + restartPolicy: Never + containers: + - name: probe + image: busybox +` + var _ = Describe("Lua Health Policies", func() { It("fetchResources returns resources", func() { @@ -329,6 +345,132 @@ end` }) }) +var _ = Describe("FetchJobManifest", func() { + logger := textlogger.NewLogger(textlogger.NewConfig()) + + var clusterNamespace string + var clusterName string + var clusterType libsveltosv1beta1.ClusterType + var sveltosCluster *libsveltosv1beta1.SveltosCluster + + BeforeEach(func() { + clusterNamespace = randomString() + clusterName = randomString() + clusterType = libsveltosv1beta1.ClusterTypeSveltos + + sveltosCluster = &libsveltosv1beta1.SveltosCluster{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: clusterName}, + } + }) + + It("decodes a Job manifest from a referenced ConfigMap", func() { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: randomString()}, + Data: map[string]string{jobManifestDataKey: jobManifest}, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sveltosCluster, configMap).Build() + + jobRef := &libsveltosv1beta1.PolicyRef{ + Namespace: configMap.Namespace, + Name: configMap.Name, + Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), + } + + job, err := clusterops.FetchJobManifest(context.TODO(), c, clusterNamespace, clusterName, clusterType, jobRef, logger) + Expect(err).To(BeNil()) + Expect(job.Name).To(Equal("probe")) + Expect(job.Spec.Template.Spec.Containers).To(HaveLen(1)) + }) + + It("decodes a Job manifest from a referenced Secret", func() { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: randomString()}, + Data: map[string][]byte{jobManifestDataKey: []byte(jobManifest)}, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sveltosCluster, secret).Build() + + jobRef := &libsveltosv1beta1.PolicyRef{ + Namespace: secret.Namespace, + Name: secret.Name, + Kind: string(libsveltosv1beta1.SecretReferencedResourceKind), + } + + job, err := clusterops.FetchJobManifest(context.TODO(), c, clusterNamespace, clusterName, clusterType, jobRef, logger) + Expect(err).To(BeNil()) + Expect(job.Name).To(Equal("probe")) + }) + + It("fails when the ConfigMap does not contain a Job manifest", func() { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: randomString()}, + Data: map[string]string{"deployment.yaml": `apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo +`}, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sveltosCluster, configMap).Build() + + jobRef := &libsveltosv1beta1.PolicyRef{ + Namespace: configMap.Namespace, + Name: configMap.Name, + Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), + } + + _, err := clusterops.FetchJobManifest(context.TODO(), c, clusterNamespace, clusterName, clusterType, jobRef, logger) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("does not contain a Job manifest")) + }) + + It("fails when the ConfigMap contains more than one entry", func() { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: randomString()}, + Data: map[string]string{ + jobManifestDataKey: jobManifest, + "other.yaml": jobManifest, + }, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sveltosCluster, configMap).Build() + + jobRef := &libsveltosv1beta1.PolicyRef{ + Namespace: configMap.Namespace, + Name: configMap.Name, + Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), + } + + _, err := clusterops.FetchJobManifest(context.TODO(), c, clusterNamespace, clusterName, clusterType, jobRef, logger) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("exactly one Job manifest")) + }) + + It("fails when the manifest has no containers", func() { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNamespace, Name: randomString()}, + Data: map[string]string{jobManifestDataKey: `apiVersion: batch/v1 +kind: Job +metadata: + name: probe +`}, + } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sveltosCluster, configMap).Build() + + jobRef := &libsveltosv1beta1.PolicyRef{ + Namespace: configMap.Namespace, + Name: configMap.Name, + Kind: string(libsveltosv1beta1.ConfigMapReferencedResourceKind), + } + + _, err := clusterops.FetchJobManifest(context.TODO(), c, clusterNamespace, clusterName, clusterType, jobRef, logger) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("no containers")) + }) +}) + func getResources(dirName, fileName string) []*unstructured.Unstructured { resourceFileName := filepath.Join(dirName, fileName) diff --git a/test/fv/job_health_check_test.go b/test/fv/job_health_check_test.go index eb2933fb..c4b89cd1 100644 --- a/test/fv/job_health_check_test.go +++ b/test/fv/job_health_check_test.go @@ -66,7 +66,7 @@ var _ = Describe("JobCheck", func() { namePrefix = "job-check-" ) - It("provisions once the referenced Job completes successfully", Label("Enterprise"), func() { + It("provisions once the referenced Job completes successfully", Label("Enterprise", "NEW-FV-PULLMODE"), func() { resourceNs := randomString() resourceName := randomString() jobName := randomString() @@ -125,7 +125,7 @@ var _ = Describe("JobCheck", func() { deleteClusterProfile(clusterProfile) }) - It("reports a failure while the referenced Job keeps failing", Label("Enterprise"), func() { + It("reports a failure while the referenced Job keeps failing", Label("Enterprise", "NEW-FV-PULLMODE"), func() { resourceNs := randomString() resourceName := randomString() jobName := randomString() diff --git a/test/pullmode-sveltosapplier.yaml b/test/pullmode-sveltosapplier.yaml index 74229052..5b2492dd 100644 --- a/test/pullmode-sveltosapplier.yaml +++ b/test/pullmode-sveltosapplier.yaml @@ -99,7 +99,7 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - image: docker.io/projectsveltos/sveltos-applier@sha256:5dda38ec232f80db07b29d23cd95bcbb7271011d3d14a6705b89460a51b15bfd + image: docker.io/projectsveltos/sveltos-applier@sha256:f51e4973b896cdedcfb6e50dc40516d7cab2dad9714eeca9730ded49421dfac5 livenessProbe: failureThreshold: 3 httpGet: