fix: ignore not existing cluster in MachineSet teardown flow

Unix4ever · Unix4ever · commit de4c096a9b99 · 2024-04-11T16:12:04.000+03:00
If the cluster is not found it's not critical for teardown flow, so we
shouldn't skip reconcile in that case.

Signed-off-by: Artem Chernyshev &lt;artem.chernyshev@talos-systems.com&gt;
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/control_planes_handler_test.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/control_planes_handler_test.go
@@ -187,6 +187,7 @@ func TestControlPlanesHandler(t *testing.T) {
 			require := require.New(t)
 
 			machineSet.TypedSpec().Value = tt.machineSet
+			machineSet.Metadata().Labels().Set(omni.LabelCluster, tt.name)
 
 			cluster := omni.NewCluster(resources.DefaultNamespace, tt.name)
 			cluster.TypedSpec().Value.TalosVersion = "v1.5.4"
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/operations.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/operations.go
@@ -47,11 +47,16 @@ func (c *Create) Apply(ctx context.Context, r controller.ReaderWriter, logger *z
 	helpers.UpdateInputsVersions(clusterMachine, configPatches...)
 	setPatches(clusterMachineConfigPatches, configPatches)
 
-	clusterMachine.TypedSpec().Value.KubernetesVersion = rc.GetCluster().TypedSpec().Value.KubernetesVersion
+	var err error
+
+	clusterMachine.TypedSpec().Value.KubernetesVersion, err = rc.GetKubernetesVersion()
+	if err != nil {
+		return err
+	}
 
 	logger.Info("create cluster machine", zap.String("machine", c.ID))
 
-	if err := r.Create(ctx, clusterMachine); err != nil {
+	if err = r.Create(ctx, clusterMachine); err != nil {
 		return err
 	}
 
@@ -78,7 +83,7 @@ func (d *Teardown) Apply(ctx context.Context, r controller.ReaderWriter, logger
 		return fmt.Errorf(
 			"error tearing down machine %q in cluster %q: %w",
 			d.ID,
-			rc.GetCluster().Metadata().ID(),
+			rc.GetClusterName(),
 			err,
 		)
 	}
@@ -135,14 +140,17 @@ func (u *Update) Apply(ctx context.Context, r controller.ReaderWriter, logger *z
 	return safe.WriterModify(ctx, r, clusterMachine, func(res *omni.ClusterMachine) error {
 		// don't update the ClusterMachine if it's still owned by another cluster
 		currentClusterName, ok := res.Metadata().Labels().Get(omni.LabelCluster)
-		if ok && currentClusterName != rc.cluster.Metadata().ID() {
+		if ok && currentClusterName != rc.GetClusterName() {
 			return nil
 		}
 
 		helpers.CopyAllAnnotations(clusterMachine, res)
 
 		if res.TypedSpec().Value.KubernetesVersion == "" {
-			res.TypedSpec().Value.KubernetesVersion = rc.GetCluster().TypedSpec().Value.KubernetesVersion
+			res.TypedSpec().Value.KubernetesVersion, err = rc.GetKubernetesVersion()
+			if err != nil {
+				return err
+			}
 		}
 
 		return nil
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/operations_test.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/operations_test.go
@@ -85,6 +85,7 @@ func TestCreate(t *testing.T) {
 	cluster.TypedSpec().Value.KubernetesVersion = "v1.6.2"
 
 	machineSet := omni.NewMachineSet(resources.DefaultNamespace, "machineset")
+	machineSet.Metadata().Labels().Set(omni.LabelCluster, cluster.Metadata().ID())
 
 	create := machineset.Create{ID: "aa"}
 
@@ -157,6 +158,7 @@ func TestUpdate(t *testing.T) {
 	cluster.TypedSpec().Value.KubernetesVersion = "v1.6.4"
 
 	machineSet := omni.NewMachineSet(resources.DefaultNamespace, "machineset")
+	machineSet.Metadata().Labels().Set(omni.LabelCluster, cluster.Metadata().ID())
 
 	quota := &machineset.ChangeQuota{
 		Update: 2,
@@ -292,6 +294,7 @@ func TestTeardown(t *testing.T) {
 	cluster.TypedSpec().Value.KubernetesVersion = "v1.6.3"
 
 	machineSet := omni.NewMachineSet(resources.DefaultNamespace, "machineset")
+	machineSet.Metadata().Labels().Set(omni.LabelCluster, cluster.Metadata().ID())
 
 	quota := machineset.ChangeQuota{
 		Teardown: 1,
@@ -358,6 +361,7 @@ func TestDestroy(t *testing.T) {
 	cluster.TypedSpec().Value.KubernetesVersion = "v1.6.3"
 
 	machineSet := omni.NewMachineSet(resources.DefaultNamespace, "machineset")
+	machineSet.Metadata().Labels().Set(omni.LabelCluster, cluster.Metadata().ID())
 
 	require := require.New(t)
 
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/reconciliation_context.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/reconciliation_context.go
@@ -7,14 +7,13 @@ package machineset
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/cosi-project/runtime/pkg/controller"
-	"github.com/cosi-project/runtime/pkg/controller/generic/qtransform"
 	"github.com/cosi-project/runtime/pkg/resource"
 	"github.com/cosi-project/runtime/pkg/safe"
 	"github.com/cosi-project/runtime/pkg/state"
-	"github.com/siderolabs/gen/xerrors"
 	"github.com/siderolabs/gen/xslices"
 
 	"github.com/siderolabs/omni/client/api/omni/specs"
@@ -76,6 +75,8 @@ type ReconciliationContext struct {
 	clusterMachineConfigPatchesMap  map[resource.ID]*omni.ClusterMachineConfigPatches
 	clusterMachineStatusesMap       map[resource.ID]*omni.ClusterMachineStatus
 
+	clusterName string
+
 	runningMachineSetNodesSet Set[string]
 	idsTearingDown            Set[string]
 	idsUnconfigured           Set[string]
@@ -104,11 +105,7 @@ func BuildReconciliationContext(
 	}
 
 	cluster, err := safe.ReaderGetByID[*omni.Cluster](ctx, r, clusterName)
-	if err != nil {
-		if state.IsNotFoundError(err) {
-			return nil, xerrors.NewTagged[qtransform.SkipReconcileTag](err)
-		}
-
+	if err != nil && !state.IsNotFoundError(err) {
 		return nil, err
 	}
 
@@ -179,9 +176,15 @@ func NewReconciliationContext(
 	clusterMachineConfigPatches []*omni.ClusterMachineConfigPatches,
 	clusterMachineStatuses []*omni.ClusterMachineStatus,
 ) (*ReconciliationContext, error) {
+	clusterName, ok := machineSet.Metadata().Labels().Get(omni.LabelCluster)
+	if !ok {
+		return nil, fmt.Errorf("failed to determine the cluster of the machine set %q", machineSet.Metadata().ID())
+	}
+
 	rc := &ReconciliationContext{
 		machineSet:       machineSet,
 		cluster:          cluster,
+		clusterName:      clusterName,
 		patchesByMachine: map[resource.ID][]*omni.ConfigPatch{},
 	}
 
@@ -345,9 +348,18 @@ func (rc *ReconciliationContext) GetOutdatedMachines() Set[string] {
 	return rc.idsOutdated
 }
 
-// GetCluster reads the related cluster resource.
-func (rc *ReconciliationContext) GetCluster() *omni.Cluster {
-	return rc.cluster
+// GetKubernetesVersion reads kubernetes version from the related cluster if the cluster exists.
+func (rc *ReconciliationContext) GetKubernetesVersion() (string, error) {
+	if rc.cluster == nil {
+		return "", errors.New("failed to get kubernetes version for the machine set as the cluster couldn't be found")
+	}
+
+	return rc.cluster.TypedSpec().Value.KubernetesVersion, nil
+}
+
+// GetClusterName reads cluster name from the context.
+func (rc *ReconciliationContext) GetClusterName() string {
+	return rc.clusterName
 }
 
 // GetMachineSet reads the related machine set resource.
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/reconciliation_context_test.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/reconciliation_context_test.go
@@ -312,6 +312,7 @@ func TestReconciliationContext(t *testing.T) {
 
 			machineSet := omni.NewMachineSet(resources.DefaultNamespace, tt.name)
 			machineSet.TypedSpec().Value = tt.machineSet
+			machineSet.Metadata().Labels().Set(omni.LabelCluster, tt.name)
 
 			cluster := omni.NewCluster(resources.DefaultNamespace, tt.name)
 			cluster.TypedSpec().Value.TalosVersion = "v1.6.4"
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/status_handler_test.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/status_handler_test.go
@@ -329,6 +329,8 @@ func TestStatusHandler(t *testing.T) {
 				machineSet = omni.NewMachineSet(resources.DefaultNamespace, "test")
 			}
 
+			machineSet.Metadata().Labels().Set(omni.LabelCluster, "test")
+
 			require := require.New(t)
 
 			clusterMachineConfigStatuses := make([]*omni.ClusterMachineConfigStatus, 0, len(tt.clusterMachines))
diff --git a/internal/backend/runtime/omni/controllers/omni/internal/machineset/workers_handler_test.go b/internal/backend/runtime/omni/controllers/omni/internal/machineset/workers_handler_test.go
@@ -173,6 +173,7 @@ func TestWorkersHandler(t *testing.T) {
 			require := require.New(t)
 
 			machineSet.TypedSpec().Value = tt.machineSet
+			machineSet.Metadata().Labels().Set(omni.LabelCluster, tt.name)
 
 			cluster := omni.NewCluster(resources.DefaultNamespace, tt.name)
 			cluster.TypedSpec().Value.TalosVersion = "v1.6.0"

Original file line number	Diff line number	Diff line change
`@@ -329,6 +329,8 @@ func TestStatusHandler(t *testing.T) {`
`329`	`329`	`machineSet = omni.NewMachineSet(resources.DefaultNamespace, "test")`
`330`	`330`	`}`
`331`	`331`
	`332`	`+ machineSet.Metadata().Labels().Set(omni.LabelCluster, "test")`
	`333`	`+`
`332`	`334`	`require := require.New(t)`
`333`	`335`
`334`	`336`	`clusterMachineConfigStatuses := make([]*omni.ClusterMachineConfigStatus, 0, len(tt.clusterMachines))`