Repository navigation
Rework on-cluster manifest #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,11 +147,11 @@ func (d *Deployer) deleteCentralResources(ctx context.Context) error { | |
| return nil | ||
| } | ||
|
|
||
| func (d *Deployer) deleteSecuredClusterResources(ctx context.Context) error { | ||
| func (d *Deployer) deleteSecuredClusterResources(ctx context.Context, namespace string) error { | ||
| log.Info("Deleting SecuredCluster resources") | ||
| crExists := true | ||
|
|
||
| if _, err := k8s.RetrieveResourceFromCluster(ctx, d.config.SecuredCluster.Namespace, "securedcluster", "stackrox-secured-cluster-services"); err != nil { | ||
| if _, err := k8s.RetrieveResourceFromCluster(ctx, namespace, "securedcluster", securedClusterCrName); err != nil { | ||
| if !k8s.IsResourceNotFound(err) { | ||
| return fmt.Errorf("retrieving SecuredCluster CR: %w", err) | ||
| } | ||
|
|
@@ -160,12 +160,12 @@ func (d *Deployer) deleteSecuredClusterResources(ctx context.Context) error { | |
|
|
||
| if crExists { | ||
| log.Info("Removing any pause-reconcile annotation from SecuredCluster") | ||
| if err := d.removePauseReconcileAnnotation(ctx, "securedcluster", "stackrox-secured-cluster-services", d.config.SecuredCluster.Namespace); err != nil { | ||
| if err := d.removePauseReconcileAnnotation(ctx, "securedcluster", securedClusterCrName, namespace); err != nil { | ||
| return err | ||
| } | ||
| log.Debug("Removed any pause-reconcile annotation from SecuredCluster") | ||
|
|
||
| err := d.deleteResource(ctx, d.config.SecuredCluster.Namespace, "securedcluster", "stackrox-secured-cluster-services", "--wait") | ||
| err := d.deleteResource(ctx, namespace, "securedcluster", securedClusterCrName, "--wait") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -183,9 +183,7 @@ func (d *Deployer) deleteSecuredClusterResources(ctx context.Context) error { | |
| } { | ||
| log.Dimf("Attempting to delete %s/%s", resource.Kind, resource.Name) | ||
| if resource.OwnerName != "" { | ||
| // Avoid deletion if the resource does not have the expected owner. | ||
| // (e.g. in case central and secured cluster are deployed into the same namespace). | ||
| obj, err := k8s.RetrieveResourceFromCluster(ctx, d.config.SecuredCluster.Namespace, resource.Kind, resource.Name) | ||
| obj, err := k8s.RetrieveResourceFromCluster(ctx, namespace, resource.Kind, resource.Name) | ||
| if err != nil { | ||
| if !k8s.IsResourceNotFound(err) { | ||
| log.Warningf("Failed to retrieve %s/%s for owner checking: %v. Skipping deletion. Deployment might be affected.", resource.Kind, resource.Name, err) | ||
|
|
@@ -197,7 +195,7 @@ func (d *Deployer) deleteSecuredClusterResources(ctx context.Context) error { | |
| continue | ||
| } | ||
| } | ||
| if err := d.deleteResource(ctx, d.config.SecuredCluster.Namespace, resource.Kind, resource.Name); err != nil { | ||
| if err := d.deleteResource(ctx, namespace, resource.Kind, resource.Name); err != nil { | ||
| return fmt.Errorf("failed to delete %s/%s: %w", resource.Kind, resource.Name, err) | ||
| } | ||
| } | ||
|
|
@@ -497,19 +495,21 @@ func (d *Deployer) Teardown(ctx context.Context, components component.Component) | |
| } | ||
|
|
||
| func (d *Deployer) teardownCentral(ctx context.Context) error { | ||
| log.Infof("🗑️ Tearing down central in namespace %s", d.config.Central.Namespace) | ||
|
|
||
| if !d.namespaceExists(d.config.Central.Namespace) { | ||
| log.Infof("Namespace %s doesn't exist, skipping", d.config.Central.Namespace) | ||
| namespaces, err := d.discoverCRNamespaces(ctx, "central", centralCrName) | ||
| if err != nil { | ||
| log.Infof("No Central CR found on cluster, skipping Central teardown") | ||
|
Comment on lines
+499
to
+500
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Propagate discovery errors instead of treating them as absent CRs. An authorization or API failure currently makes either teardown return success without deleting resources. Skip only when a successful discovery query finds no CRs. (kubernetes.io)
📍 Affects 1 file
🤖 Prompt for AI Agents |
||
| return nil | ||
|
Comment on lines
+499
to
501
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Keep a cleanup path when a CR has already been deleted. A missing CR no longer identifies a namespace, but the namespace can still hold resources that teardown previously removed. (kubernetes.io)
📍 Affects 1 file
🤖 Prompt for AI Agents |
||
| } | ||
| d.config.Central.Namespace = namespaces[0] | ||
|
|
||
| log.Infof("🗑️ Tearing down central in namespace %s", d.config.Central.Namespace) | ||
|
|
||
| d.portForward.Stop() | ||
| d.stopDetachedPortForward() | ||
|
|
||
| // Add pause-reconcile annotation to not have the operator interfere during resource deletion. | ||
| if d.doesResourceExist(ctx, "central", "stackrox-central-services", d.config.Central.Namespace) { | ||
| if err := d.addPauseReconcileAnnotation(ctx, "central", "stackrox-central-services", d.config.Central.Namespace); err != nil { | ||
| if d.doesResourceExist(ctx, "central", centralCrName, d.config.Central.Namespace) { | ||
| if err := d.addPauseReconcileAnnotation(ctx, "central", centralCrName, d.config.Central.Namespace); err != nil { | ||
| log.Warningf("Error adding pause-reconcile annotation: %v", err) | ||
| } | ||
| } | ||
|
|
@@ -524,27 +524,41 @@ func (d *Deployer) teardownCentral(ctx context.Context) error { | |
| } | ||
|
|
||
| func (d *Deployer) teardownSecuredCluster(ctx context.Context) error { | ||
| log.Infof("🗑️ Tearing down secured cluster in namespace %s", d.config.SecuredCluster.Namespace) | ||
|
|
||
| if !d.namespaceExists(d.config.SecuredCluster.Namespace) { | ||
| log.Infof("Namespace %s doesn't exist, skipping", d.config.SecuredCluster.Namespace) | ||
| namespaces, err := d.discoverCRNamespaces(ctx, "securedcluster", securedClusterCrName) | ||
| if err != nil { | ||
| log.Infof("No SecuredCluster CR found on cluster, skipping SecuredCluster teardown") | ||
| return nil | ||
| } | ||
|
|
||
| if d.doesResourceExist(ctx, "securedcluster", "stackrox-secured-cluster-services", d.config.SecuredCluster.Namespace) { | ||
| // Add pause-reconcile annotation to not have the operator interfere during resource deletion. | ||
| if err := d.addPauseReconcileAnnotation(ctx, "securedcluster", "stackrox-secured-cluster-services", d.config.SecuredCluster.Namespace); err != nil { | ||
| var wg sync.WaitGroup | ||
| wg.Add(len(namespaces)) | ||
| for _, ns := range namespaces { | ||
| go func() { | ||
| defer wg.Done() | ||
| if err := d.teardownSecuredClusterInNamespace(ctx, ns); err != nil { | ||
| log.Warningf("Error tearing down SecuredCluster in namespace %s: %v", ns, err) | ||
| } | ||
|
Comment on lines
+538
to
+540
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Return per-namespace teardown errors. If deletion fails in one namespace, this goroutine only logs the error. After 🤖 Prompt for AI Agents |
||
| }() | ||
| } | ||
| wg.Wait() | ||
| return nil | ||
| } | ||
|
|
||
| func (d *Deployer) teardownSecuredClusterInNamespace(ctx context.Context, namespace string) error { | ||
| log.Infof("🗑️ Tearing down secured cluster in namespace %s", namespace) | ||
|
|
||
| if d.doesResourceExist(ctx, "securedcluster", securedClusterCrName, namespace) { | ||
| if err := d.addPauseReconcileAnnotation(ctx, "securedcluster", securedClusterCrName, namespace); err != nil { | ||
| log.Warningf("Error adding pause-reconcile annotation: %v", err) | ||
| } | ||
| } | ||
|
|
||
| log.Info("⏳ Waiting for SecuredCluster resources to be fully deleted...") | ||
| err := d.deleteSecuredClusterResources(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to delete SecuredCluster resources: %w", err) | ||
| if err := d.deleteSecuredClusterResources(ctx, namespace); err != nil { | ||
| return fmt.Errorf("failed to delete SecuredCluster resources in namespace %s: %w", namespace, err) | ||
| } | ||
|
|
||
| log.Successf("✓ SecuredCluster resources in namespace %s have been deleted", d.config.SecuredCluster.Namespace) | ||
| log.Successf("✓ SecuredCluster resources in namespace %s have been deleted", namespace) | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -646,6 +660,21 @@ func (d *Deployer) SetEnvrcFile(path string) { | |
| d.envrcFile = path | ||
| } | ||
|
|
||
| func (d *Deployer) discoverCRNamespaces(ctx context.Context, crType, crName string) ([]string, error) { | ||
| result, err := k8s.RunKubectl(ctx, k8s.KubectlOptions{ | ||
| Args: []string{"get", crType, crName, "-A", "-o", "jsonpath={.items[*].metadata.namespace}"}, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win List CRs before filtering by name.
🤖 Prompt for AI Agents |
||
| MaxAttempts: 1, | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("discovering %s/%s: %w", crType, crName, err) | ||
| } | ||
| raw := strings.TrimSpace(result.Stdout) | ||
| if raw == "" { | ||
| return nil, fmt.Errorf("%s/%s not found on cluster", crType, crName) | ||
| } | ||
| return strings.Fields(raw), nil | ||
| } | ||
|
|
||
| func (d *Deployer) doesResourceExist(ctx context.Context, resourceType, resourceName, namespace string) bool { | ||
| _, err := k8s.RunKubectl(ctx, k8s.KubectlOptions{ | ||
| Args: []string{ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,36 @@ const ( | |
| // We include the whole deployer Config for reproducibility purposes. | ||
| type RoxieManifest struct { | ||
| RoxieEnvironment types.RoxieEnvironment `yaml:"roxieEnvironment"` | ||
| Config deployer.Config `yaml:"config"` | ||
| Config RoxieManifestConfig `yaml:"config"` | ||
| } | ||
|
|
||
| // RoxieManifestConfig stores the subset of deployer.Config that is persisted | ||
| // in the cluster manifest secret. Namespaces are NOT consumed from here — | ||
| // teardown discovers them directly from CRs on the cluster. The fields that | ||
| // are actively used: | ||
| // | ||
| // - Central.AddOns / Central.AvailableAddOns — teardown needs these to know | ||
| // which add-ons were deployed so it can tear them down (add-ons are | ||
| // lifecycle-aligned with Central and not discoverable from the cluster). | ||
| // - Central.Namespace — used by 'roxie shell' to fetch the CA cert from the | ||
| // central-tls secret, and to resolve the Central endpoint. | ||
| // - Central.Exposure — used by 'roxie shell' to determine the base URL shown | ||
| // in the subshell banner. | ||
| // - Roxie.HAProxy — used by 'roxie shell' to re-spawn the HAProxy sidecar | ||
| // for local HTTP access to Central. | ||
| type RoxieManifestConfig struct { | ||
| Roxie deployer.RoxieConfig `yaml:"roxie"` | ||
| Central deployer.CentralConfig `yaml:"central"` | ||
| } | ||
|
|
||
| func (c *RoxieManifestConfig) AsConfig() *deployer.Config { | ||
| if c == nil { | ||
| return nil | ||
| } | ||
| return &deployer.Config{ | ||
| Roxie: c.Roxie, | ||
| Central: c.Central, | ||
| } | ||
|
Comment on lines
+57
to
+60
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Restore defaults for configuration sections omitted from the manifest. When teardown loads a manifest, 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| func manifestToSecret(m RoxieManifest) (*unstructured.Unstructured, error) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.