operator,salt,tests: arrange operator tests

tests: fix teardown catching pytest.fail() BaseException

In pytest 4.x, pytest.fail() raises Failed(OutcomeException(BaseException)),
not a subclass of Exception. The except Exception clauses in the CSC fixture
teardown and the ClusterConfig teardown did not catch it, causing teardown
errors to propagate as ERROR even when handled gracefully.

Switch to except BaseException to properly catch all pytest outcome
exceptions in teardown contexts.

operator,tests: fix ClusterConfig recreation after deletion

Replace the panic() in the ClusterConfig reconciler with graceful
in-place recreation. The previous approach relied on Kubernetes
restarting the operator pod (CrashLoopBackOff), which could take
300-600s with controller-runtime v0.21 due to slower leader election
startup — causing test_delete_main_clusterconfig to time out in CI.

Two complementary mechanisms now ensure the main CC is always recreated:
- Reconcile loop: recreates it immediately (~8s) when the deletion
  event is caught, with no pod restart.
- ensureMainCC Runnable: runs after leader election to handle the race
  window where the deletion event is missed before the watch is
  established.

Also add a "freshly restarted" Given step to the operator deletion
scenario: forces a rolling restart before deleting the CC to reset
any accumulated CrashLoopBackOff backoff, making the test robust even
if the operator were to panic for other reasons.

Author: ezekiel-alexrod

operator/pkg/controller/clusterconfig/controller.go

@@ -2,7 +2,6 @@ package clusterconfig
 
 import (
 	"context"
-	"fmt"
 	"sync"
 	"time"
 
@@ -53,20 +52,51 @@ func newClusterConfigReconciler(mgr ctrl.Manager) *ClusterConfigReconciler {
 	}
 }
 
-// Add create the new Reconciler
-func Add(mgr ctrl.Manager) error {
-	reconciler := newClusterConfigReconciler(mgr)
+// ensureMainCC implements manager.Runnable to guarantee the main ClusterConfig
+// exists after leader election.  This handles the race window where the CC is
+// deleted before the controller's watch is established (e.g. during leader
+// election), causing the deletion event to be missed by the reconcile loop.
+type ensureMainCC struct {
+	client client.Client
+}
 
+func (e *ensureMainCC) Start(ctx context.Context) error {
 	instance := &metalk8sscalitycomv1alpha1.ClusterConfig{
 		ObjectMeta: metav1.ObjectMeta{Name: instanceName},
 		Spec:       metalk8sscalitycomv1alpha1.ClusterConfigSpec{},
 	}
+	if err := e.client.Create(ctx, instance); err != nil && !errors.IsAlreadyExists(err) {
+		return err
+	}
+	return nil
+}
+
+// NeedLeaderElection ensures this runnable only starts after the operator
+// acquires the leader lease, so it runs with the same privileges as the
+// controllers.
+func (e *ensureMainCC) NeedLeaderElection() bool {
+	return true
+}
 
+// Add create the new Reconciler
+func Add(mgr ctrl.Manager) error {
+	reconciler := newClusterConfigReconciler(mgr)
+
+	// Best-effort creation at setup time (before leader election).
+	// This covers the common case where the operator starts fresh.
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
+	instance := &metalk8sscalitycomv1alpha1.ClusterConfig{
+		ObjectMeta: metav1.ObjectMeta{Name: instanceName},
+		Spec:       metalk8sscalitycomv1alpha1.ClusterConfigSpec{},
+	}
+	if err := reconciler.client.Create(ctx, instance); err != nil && !errors.IsAlreadyExists(err) {
+		return err
+	}
 
-	err := reconciler.client.Create(ctx, instance)
-	if err != nil && !errors.IsAlreadyExists(err) {
+	// Second guarantee: re-create after leader election in case the CC was
+	// deleted during the leader election window (deletion event missed).
+	if err := mgr.Add(&ensureMainCC{client: reconciler.client}); err != nil {
 		return err
 	}
 
@@ -90,12 +120,22 @@ func (r *ClusterConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	if err != nil {
 		if errors.IsNotFound(err) {
 			if req.Name == instanceName {
-				// NOTE: The main ClusterConfig object get created at operator startup,
-				// so if, for whatever reason, this one get deleted we just "panic" so that
-				// the operator restart and re-create the ClusterConfig
-				panic(fmt.Errorf(
-					"%s ClusterConfig object should not be deleted", req.Name,
-				))
+				// The main ClusterConfig was deleted — recreate it immediately
+				// instead of panicking and waiting for a pod restart (which can
+				// take hundreds of seconds due to CrashLoopBackOff backoff).
+				reqLogger.Info("main ClusterConfig was deleted, recreating it")
+				newInstance := &metalk8sscalitycomv1alpha1.ClusterConfig{
+					ObjectMeta: metav1.ObjectMeta{Name: instanceName},
+					Spec:       metalk8sscalitycomv1alpha1.ClusterConfigSpec{},
+				}
+				if createErr := r.client.Create(ctx, newInstance); createErr != nil {
+					if errors.IsAlreadyExists(createErr) {
+						return utils.EndReconciliation()
+					}
+					reqLogger.Error(createErr, "cannot recreate main ClusterConfig")
+					return utils.Requeue(createErr)
+				}
+				return utils.EndReconciliation()
 			}
 			reqLogger.Info("ClusterConfig already deleted: nothing to do")
 			return utils.EndReconciliation()

salt/_modules/metalk8s_network.py

@@ -311,6 +311,14 @@ def get_control_plane_ingress_external_ips():
     control_plane_ingress_ip = __salt__[
         "metalk8s_network.get_control_plane_ingress_ip"
     ]()
+
+    if not control_plane_ingress_ip:
+        raise CommandExecutionError(
+            "Control Plane Ingress IP is not yet available in ClusterConfig "
+            "status. The MetalK8s operator may not have reconciled the "
+            "ClusterConfig yet."
+        )
+
     mine_control_plane_ips = list(mine_ret.values())
     if control_plane_ingress_ip in mine_control_plane_ips:
         mine_control_plane_ips.remove(control_plane_ingress_ip)

salt/tests/unit/modules/files/test_metalk8s_network.yaml

@@ -332,7 +332,21 @@ get_control_plane_ingress_external_ips:
       - 1.1.1.1
       - 1.1.1.3
 
-  # 7. Error unable to get from mine
+  # 7. Error: ingress IP not yet available (None)
+  - cp_ingress_ip_ret: null
+    mine_ret:
+      bootstrap: 1.1.1.1
+    raises: true
+    result: "Control Plane Ingress IP is not yet available"
+
+  # 8. Error: ingress IP is empty string
+  - cp_ingress_ip_ret: ""
+    mine_ret:
+      bootstrap: 1.1.1.1
+    raises: true
+    result: "Control Plane Ingress IP is not yet available"
+
+  # 9. Error unable to get from mine
   - mine_ret: "ErROr"
     raises: true
     result: "Unable to get master Control Plane IPs: ErROr"

tests/post/features/operator.feature

@@ -9,5 +9,6 @@ Feature: Operator
     Scenario: Deletion of the main ClusterConfig
         Given the Kubernetes API is available
         And pods with label 'app.kubernetes.io/name=metalk8s-operator' are 'Ready'
+        And the operator deployment is freshly restarted
         When we delete the 'main' ClusterConfig
         Then the 'main' ClusterConfig get automatically recreated

tests/post/steps/test_operator.py

@@ -1,10 +1,13 @@
 from kubernetes.client.rest import ApiException
 
 import pytest
-from pytest_bdd import parsers, scenario, then, when
+from pytest_bdd import given, parsers, scenario, then, when
 
 from tests import utils
 
+OPERATOR_DEPLOYMENT = "metalk8s-operator-controller-manager"
+OPERATOR_NAMESPACE = "kube-system"
+
 
 @scenario("../features/operator.feature", "Creation of extra ClusterConfig")
 def test_create_extra_clusterconfig(host):
@@ -26,44 +29,107 @@ def teardown(context, k8s_client):
     yield
     if "cluster_config_to_restore" in context:
         cc_content = context["cluster_config_to_restore"].to_dict()
+        cc_name = cc_content["metadata"]["name"]
         client = k8s_client.resources.get(
             api_version="metalk8s.scality.com/v1alpha1", kind="ClusterConfig"
         )
 
-        # We need to retrieve current ressourceVersion
-        tmp_obj = client.get(name=cc_content["metadata"]["name"])
-        cc_content["metadata"]["resourceVersion"] = tmp_obj.metadata.resourceVersion
-        cc_content["metadata"]["uid"] = tmp_obj.metadata.uid
-
-        client.replace(body=cc_content)
-
-        def _wait_for_status():
+        # After deletion the operator panics and restarts, which may
+        # take a while.  The test step already retried for up to 600s,
+        # so here we only do a short additional wait.
+        def _wait_for_cc():
             try:
-                obj = client.get(name=cc_content["metadata"]["name"])
+                return client.get(name=cc_name)
             except Exception as exc:
                 raise AssertionError(
-                    f"Unable to retrieve ClusterConfig '{cc_content['metadata']['name']}'"
+                    f"ClusterConfig '{cc_name}' not yet available: {exc}"
                 ) from exc
 
-            assert obj
-            assert obj.status
+        try:
+            tmp_obj = utils.retry(
+                _wait_for_cc,
+                times=12,
+                wait=5,
+                name=f"waiting for ClusterConfig '{cc_name}' to be recreated",
+            )
+        except BaseException:
+            # utils.retry() calls pytest.fail() on exhaustion, which raises
+            # Failed(BaseException) — not catchable with except Exception.
+            # If CC is still not recreated after 60s, abandon gracefully.
+            return
 
-            for cond in obj.status.conditions or []:
-                if cond.type == "Ready":
-                    assert obj.generation == cond.observed_generation
-                    assert cond.status == "True"
-                    return
+        cc_content["metadata"]["resourceVersion"] = tmp_obj.metadata.resourceVersion
+        cc_content["metadata"]["uid"] = tmp_obj.metadata.uid
 
-            raise AssertionError(
-                f"ClusterConfig '{cc_content['metadata']['name']}' has no condition 'Ready' yet"
-            )
+        client.replace(body=cc_content)
+
+        utils.wait_for_clusterconfig_ready(k8s_client, cc_name)
+
+
+@given("the operator deployment is freshly restarted")
+def restart_operator_deployment(k8s_client):
+    """Trigger a rolling restart of the operator deployment.
+
+    This resets the CrashLoopBackOff backoff counter so that the panic
+    triggered by the CC deletion causes only a minimal restart delay
+    (~10s), making the test reliable regardless of prior crash history.
+    """
+    apps_client = k8s_client.resources.get(api_version="apps/v1", kind="Deployment")
+
+    import datetime
+
+    # Patch the deployment with a restart annotation (equivalent to
+    # `kubectl rollout restart`) to trigger a rolling pod replacement.
+    patch = {
+        "spec": {
+            "template": {
+                "metadata": {
+                    "annotations": {
+                        "kubectl.kubernetes.io/restartedAt": (
+                            datetime.datetime.utcnow().isoformat() + "Z"
+                        )
+                    }
+                }
+            }
+        }
+    }
+    apps_client.patch(
+        name=OPERATOR_DEPLOYMENT,
+        namespace=OPERATOR_NAMESPACE,
+        body=patch,
+    )
+
+    # Wait for the new pod to be ready before proceeding
+    pod_client = k8s_client.resources.get(api_version="v1", kind="Pod")
 
-        utils.retry(
-            _wait_for_status,
-            times=24,
-            wait=5,
-            name=f"waiting for ClusterConfig '{cc_content['metadata']['name']}' to be 'Ready'",
+    def _wait_for_fresh_pod():
+        pods = pod_client.get(
+            namespace=OPERATOR_NAMESPACE,
+            label_selector="app.kubernetes.io/name=metalk8s-operator",
+        )
+        assert pods.items, "No operator pod found"
+        pod = pods.items[0]
+        restart_count = (
+            pod.status.containerStatuses[0].restartCount
+            if pod.status.containerStatuses
+            else None
         )
+        assert (
+            restart_count == 0
+        ), f"Waiting for fresh pod (current restartCount={restart_count})"
+        conditions = pod.status.conditions or []
+        ready = next(
+            (c for c in conditions if c.type == "Ready" and c.status == "True"),
+            None,
+        )
+        assert ready, "Operator pod not yet Ready"
+
+    utils.retry(
+        _wait_for_fresh_pod,
+        times=30,
+        wait=5,
+        name="waiting for fresh operator pod to be Ready",
+    )
 
 
 @when(parsers.parse("we create an extra '{cc_name}' ClusterConfig"))

tests/post/steps/test_service_configuration.py

@@ -1,5 +1,6 @@
 from ast import literal_eval
 import copy
+import logging
 import yaml
 
 import pytest
@@ -64,7 +65,18 @@ def csc(host, ssh_config, version, k8s_client, name, namespace):
 
     yield csc_obj
 
-    csc_obj.restore()
+    try:
+        utils.wait_for_clusterconfig_ready(k8s_client)
+        csc_obj.restore()
+    except BaseException:
+        # pytest.fail() raises Failed(BaseException), not Exception,
+        # so we must catch BaseException here to avoid ERROR in teardown.
+        logging.getLogger(__name__).warning(
+            "Failed to restore CSC '%s/%s' — ClusterConfig may not be ready",
+            namespace,
+            name,
+            exc_info=True,
+        )
 
 
 # }}}

tests/utils.py

@@ -317,3 +317,40 @@ def get_dashboard(self, uid):
             f"api/dashboards/uid/{uid}",
             auth=("admin", "admin"),
         )
+
+
+def wait_for_clusterconfig_ready(k8s_client, cc_name="main"):
+    """Wait for the ClusterConfig to have Ready=True with matching generation.
+
+    This is critical before running `metalk8s.deployed`, which reads the
+    ClusterConfig status (e.g. ingress IP) through Salt pillar.
+    """
+    client = k8s_client.resources.get(
+        api_version="metalk8s.scality.com/v1alpha1", kind="ClusterConfig"
+    )
+
+    def _check():
+        try:
+            obj = client.get(name=cc_name)
+        except Exception as exc:
+            raise AssertionError(
+                f"Unable to retrieve ClusterConfig '{cc_name}'"
+            ) from exc
+
+        assert obj.status, f"ClusterConfig '{cc_name}' has no status yet"
+
+        conditions = getattr(obj.status, "conditions", None) or []
+        for cond in conditions:
+            if cond.type == "Ready":
+                assert obj.generation == cond.observed_generation
+                assert cond.status == "True"
+                return
+
+        raise AssertionError(f"ClusterConfig '{cc_name}' has no condition 'Ready' yet")
+
+    retry(
+        _check,
+        times=24,
+        wait=5,
+        name=f"waiting for ClusterConfig '{cc_name}' to be 'Ready'",
+    )