diff --git a/Makefile b/Makefile index 6184ee3..6e79057 100644 --- a/Makefile +++ b/Makefile @@ -9,13 +9,13 @@ IMAGE_REPO ?= ghcr.io/oxidecomputer/cluster-api-provider-oxide HELM_OCI_REPO ?= $(IMAGE_REPO)/helm-charts IMAGE_TAG ?= dev IMG ?= $(IMAGE_REPO):$(IMAGE_TAG) -KO_DOCKER_REPO ?= $(IMAGE_REPO) +export KO_DOCKER_REPO ?= $(IMAGE_REPO) KOCACHE ?= ~/.ko MAKEFILE_PATH := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) TOOLS_MOD := $(MAKEFILE_PATH)/tools/go.mod GO_TOOL := go tool -modfile=$(TOOLS_MOD) NAMESPACE ?= capox-system -HELM_VERSION ?= v4.2.2 +HELM_VERSION ?= v4.2.3 # CAPI contract version implemented by the provider; stamped on every release # series entry when hack/gen-capi-metadata.sh generates metadata.yaml. Bump it # (and add a series boundary in the script) when migrating to a new contract. @@ -168,7 +168,7 @@ export PATH := $(LOCALBIN):$(PATH) CONTROLLER_GEN ?= $(GO_TOOL) controller-gen KUBECTL ?= $(LOCALBIN)/kubectl KIND ?= $(GO_TOOL) kind -KO ?= KO_CACHE=$(KO_CACHE) $(GO_TOOL) ko +KO ?= KOCACHE=$(KOCACHE) $(GO_TOOL) ko KUSTOMIZE ?= $(GO_TOOL) kustomize GORELEASER ?= $(GO_TOOL) goreleaser ENVTEST ?= go tool setup-envtest # this tool is in the main go.mod so the version stays in-sync @@ -219,8 +219,8 @@ golangci-lint: IMAGE_REF_FILE := $(shell echo "$${TMPDIR:-/tmp}")/capox-image-ref .PHONY: build -build: generate ## Builds a container image using ko and pushes to $KO_DOCKER_REPO - $(KO) build ./cmd | tee $(IMAGE_REF_FILE) +build: generate ## Builds a container image using ko and pushes it to $IMAGE_REPO:$IMAGE_TAG + $(KO) build --bare --tags $(IMAGE_TAG) ./cmd | tee $(IMAGE_REF_FILE) .PHONY: build-kind build-kind: @@ -290,8 +290,8 @@ deploy: generate ## Deploy controller to the K8s cluster specified in ~/.kube/co # Internal: deploy whatever image ref a build target captured in # $(IMAGE_REF_FILE). The chart renders "repository:tag", so splitting the ref -# on its last colon recomposes to the exact ref ko printed — a digest ref -# (repo@sha256 + hex) for registry builds, a digest-hex tag for --local +# on its last colon recomposes to the exact ref ko printed — a tag+digest ref +# (repo:tag@sha256 + hex) for registry builds, a digest-hex tag for kind # builds. Either way the ref is unique per build, so every deploy rolls the # pods. .PHONY: helm-apply diff --git a/api/v1alpha1/oxidecluster_types.go b/api/v1alpha1/oxidecluster_types.go index e3f3317..8ad9009 100644 --- a/api/v1alpha1/oxidecluster_types.go +++ b/api/v1alpha1/oxidecluster_types.go @@ -57,6 +57,19 @@ type OxideClusterSpec struct { // +optional // +kubebuilder:validation:Enum=v4;v6 IPType string `json:"ipType,omitempty"` + + // ControlPlaneAntiAffinityPolicy, when set, makes the controller manage an anti-affinity + // group for the cluster's control plane machines so that their instances are spread across + // sleds. The group is created in the cluster's project before the cluster is marked + // provisioned, joined by every control plane instance at creation, and deleted with the + // cluster. "allow" permits co-location when the spread cannot be satisfied; "fail" refuses + // to place the instance instead. Note that the policy is applied when the group is first + // created: the Oxide API doesn't support updating a group's policy, so changing this field + // doesn't affect an existing group, and instances only join the group as they are created. + // https://docs.oxide.computer/guides/deploying-workloads#_affinity_and_anti_affinity + // +optional + // +kubebuilder:validation:Enum=allow;fail + ControlPlaneAntiAffinityPolicy string `json:"controlPlaneAntiAffinityPolicy,omitempty"` } // SecretReference is a reference to a Secret containing Oxide credentials. Adapted from diff --git a/api/v1alpha1/oxidemachine_types.go b/api/v1alpha1/oxidemachine_types.go index b5e70df..8ce9dd5 100644 --- a/api/v1alpha1/oxidemachine_types.go +++ b/api/v1alpha1/oxidemachine_types.go @@ -43,8 +43,10 @@ type OxideMachineSpec struct { ImageID string `json:"imageID"` // AntiAffinityGroups is the list of anti-affinity group names or IDs that the Oxide instance - // should use. Anti-affinity groups should be created outside the operator, else instance - // creation will fail. + // should use. Anti-affinity groups specified should already be created, else instance + // creation will fail. A control plane anti-affinity group managed via the OxideCluster's + // ControlPlaneAntiAffinityPolicy field is joined automatically and doesn't need to be listed + // here. // https://docs.oxide.computer/guides/deploying-workloads#_affinity_and_anti_affinity // +optional // +listType=set diff --git a/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxideclusters.yaml b/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxideclusters.yaml index 3f63cf0..27f8a5d 100644 --- a/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxideclusters.yaml +++ b/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxideclusters.yaml @@ -52,6 +52,21 @@ spec: spec: description: spec defines the desired state of OxideCluster properties: + controlPlaneAntiAffinityPolicy: + description: |- + ControlPlaneAntiAffinityPolicy, when set, makes the controller manage an anti-affinity + group for the cluster's control plane machines so that their instances are spread across + sleds. The group is created in the cluster's project before the cluster is marked + provisioned, joined by every control plane instance at creation, and deleted with the + cluster. "allow" permits co-location when the spread cannot be satisfied; "fail" refuses + to place the instance instead. Note that the policy is applied when the group is first + created: the Oxide API doesn't support updating a group's policy, so changing this field + doesn't affect an existing group, and instances only join the group as they are created. + https://docs.oxide.computer/guides/deploying-workloads#_affinity_and_anti_affinity + enum: + - allow + - fail + type: string controlPlaneEndpoint: description: |- ControlPlaneEndpoint represents the host and port of the cluster's control plane. If diff --git a/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachines.yaml b/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachines.yaml index 7e90748..a35a6be 100644 --- a/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachines.yaml +++ b/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachines.yaml @@ -61,8 +61,10 @@ spec: antiAffinityGroups: description: |- AntiAffinityGroups is the list of anti-affinity group names or IDs that the Oxide instance - should use. Anti-affinity groups should be created outside the operator, else instance - creation will fail. + should use. Anti-affinity groups specified should already be created, else instance + creation will fail. A control plane anti-affinity group managed via the OxideCluster's + ControlPlaneAntiAffinityPolicy field is joined automatically and doesn't need to be listed + here. https://docs.oxide.computer/guides/deploying-workloads#_affinity_and_anti_affinity items: type: string diff --git a/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachinetemplates.yaml b/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachinetemplates.yaml index e8bbcb7..daf7968 100644 --- a/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachinetemplates.yaml +++ b/charts/cluster-api-provider-oxide/crds/infrastructure.cluster.x-k8s.io_oxidemachinetemplates.yaml @@ -79,8 +79,10 @@ spec: antiAffinityGroups: description: |- AntiAffinityGroups is the list of anti-affinity group names or IDs that the Oxide instance - should use. Anti-affinity groups should be created outside the operator, else instance - creation will fail. + should use. Anti-affinity groups specified should already be created, else instance + creation will fail. A control plane anti-affinity group managed via the OxideCluster's + ControlPlaneAntiAffinityPolicy field is joined automatically and doesn't need to be listed + here. https://docs.oxide.computer/guides/deploying-workloads#_affinity_and_anti_affinity items: type: string diff --git a/go.mod b/go.mod index e477d9c..064349f 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/oxidecomputer/cluster-api-provider-oxide -go 1.26.4 +go 1.26.5 require ( github.com/google/uuid v1.6.0 diff --git a/internal/cloud/mock/mock_client.go b/internal/cloud/mock/mock_client.go index f9532fe..5ca8694 100644 --- a/internal/cloud/mock/mock_client.go +++ b/internal/cloud/mock/mock_client.go @@ -41,6 +41,50 @@ func (m *MockOxideClient) EXPECT() *MockOxideClientMockRecorder { return m.recorder } +// AntiAffinityGroupCreate mocks base method. +func (m *MockOxideClient) AntiAffinityGroupCreate(arg0 context.Context, arg1 oxide.AntiAffinityGroupCreateParams) (*oxide.AntiAffinityGroup, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AntiAffinityGroupCreate", arg0, arg1) + ret0, _ := ret[0].(*oxide.AntiAffinityGroup) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AntiAffinityGroupCreate indicates an expected call of AntiAffinityGroupCreate. +func (mr *MockOxideClientMockRecorder) AntiAffinityGroupCreate(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AntiAffinityGroupCreate", reflect.TypeOf((*MockOxideClient)(nil).AntiAffinityGroupCreate), arg0, arg1) +} + +// AntiAffinityGroupDelete mocks base method. +func (m *MockOxideClient) AntiAffinityGroupDelete(arg0 context.Context, arg1 oxide.AntiAffinityGroupDeleteParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AntiAffinityGroupDelete", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// AntiAffinityGroupDelete indicates an expected call of AntiAffinityGroupDelete. +func (mr *MockOxideClientMockRecorder) AntiAffinityGroupDelete(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AntiAffinityGroupDelete", reflect.TypeOf((*MockOxideClient)(nil).AntiAffinityGroupDelete), arg0, arg1) +} + +// AntiAffinityGroupView mocks base method. +func (m *MockOxideClient) AntiAffinityGroupView(arg0 context.Context, arg1 oxide.AntiAffinityGroupViewParams) (*oxide.AntiAffinityGroup, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AntiAffinityGroupView", arg0, arg1) + ret0, _ := ret[0].(*oxide.AntiAffinityGroup) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AntiAffinityGroupView indicates an expected call of AntiAffinityGroupView. +func (mr *MockOxideClientMockRecorder) AntiAffinityGroupView(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AntiAffinityGroupView", reflect.TypeOf((*MockOxideClient)(nil).AntiAffinityGroupView), arg0, arg1) +} + // DiskDelete mocks base method. func (m *MockOxideClient) DiskDelete(arg0 context.Context, arg1 oxide.DiskDeleteParams) error { m.ctrl.T.Helper() diff --git a/internal/cloud/oxide.go b/internal/cloud/oxide.go index 9d693dc..72b03f0 100644 --- a/internal/cloud/oxide.go +++ b/internal/cloud/oxide.go @@ -40,6 +40,16 @@ type OxideClient interface { DiskView(context.Context, oxide.DiskViewParams) (*oxide.Disk, error) DiskDelete(context.Context, oxide.DiskDeleteParams) error + + AntiAffinityGroupCreate( + context.Context, + oxide.AntiAffinityGroupCreateParams, + ) (*oxide.AntiAffinityGroup, error) + AntiAffinityGroupView( + context.Context, + oxide.AntiAffinityGroupViewParams, + ) (*oxide.AntiAffinityGroup, error) + AntiAffinityGroupDelete(context.Context, oxide.AntiAffinityGroupDeleteParams) error } const ( diff --git a/internal/controller/naming.go b/internal/controller/naming.go index 8366896..e69ae22 100644 --- a/internal/controller/naming.go +++ b/internal/controller/naming.go @@ -11,11 +11,12 @@ import ( const maxResourceLength = 63 const ( - instancePrefix = "capi" - nicPrefix = "capi" - bootDiskPrefix = "capi-boot" - floatingIPPrefix = "capi-fip" - dataDiskPrefix = "capi-data-%d" + instancePrefix = "capi" + nicPrefix = "capi" + bootDiskPrefix = "capi-boot" + floatingIPPrefix = "capi-fip" + dataDiskPrefix = "capi-data-%d" + antiAffinityGroupPrefix = "capi-aag" ) func hashTruncateName(name string, maxLength int) string { @@ -56,3 +57,7 @@ func getFloatingIPName(obj metav1.Object) string { func getDataDiskName(obj metav1.Object, idx int) string { return getResourceName(fmt.Sprintf(dataDiskPrefix, idx), obj) } + +func getAntiAffinityGroupName(obj metav1.Object) string { + return getResourceName(antiAffinityGroupPrefix, obj) +} diff --git a/internal/controller/oxidecluster_controller.go b/internal/controller/oxidecluster_controller.go index 43ac99a..8cbc8f0 100644 --- a/internal/controller/oxidecluster_controller.go +++ b/internal/controller/oxidecluster_controller.go @@ -113,11 +113,7 @@ func (r *OxideClusterReconciler) Reconcile( ipName := getFloatingIPName(oxideCluster) if !oxideCluster.DeletionTimestamp.IsZero() { - if err := r.ensureFloatingIPDeleted(ctx, oxideClient, projectName, ipName); err != nil { - return ctrl.Result{}, fmt.Errorf("deleting floating ip: %w", err) - } - controllerutil.RemoveFinalizer(oxideCluster, infrav1.ClusterFinalizer) - return ctrl.Result{}, retErr + return ctrl.Result{}, r.reconcileDelete(ctx, oxideClient, oxideCluster, projectName, ipName) } else if !cluster.DeletionTimestamp.IsZero() { log.Info( "cluster is being deleted, aborting OxideCluster reconcile", @@ -140,10 +136,23 @@ func (r *OxideClusterReconciler) Reconcile( oxideCluster.Spec.ControlPlaneEndpoint.Port = 6443 } + // Reconcile the control plane anti-affinity group, if enabled. This must happen before the + // cluster is marked provisioned below: upstream CAPI controllers won't create Machines until + // then, and instance creation fails if the machine references a group that doesn't exist. + if err := r.reconcileAntiAffinityGroup( + ctx, + oxideClient, + oxideCluster, + projectName, + ); err != nil { + return ctrl.Result{}, fmt.Errorf("ensuring anti-affinity group: %w", err) + } + // We consider the OxideCluster to be Ready when the Oxide infrastructure that it manages (i.e. - // the floating IP) is provisioned. Note that upstream CAPI controllers won't provision - // Machine/OxideMachine resources until the OxideCluster is Ready, so we must mark Ready here - // rather than waiting for OxideMachines to be provisioned and attached. + // the floating IP and control plane anti-affinity group) is provisioned. Note that upstream + // CAPI controllers won't provision Machine/OxideMachine resources until the OxideCluster is + // Ready, so we must mark Ready here rather than waiting for OxideMachines to be provisioned + // and attached. oxideCluster.Status.Initialization.Provisioned = new(true) conditions.Set(oxideCluster, metav1.Condition{ Type: clusterv1.ReadyCondition, @@ -345,6 +354,115 @@ func (r *OxideClusterReconciler) ensureFloatingIPExists( return ip, nil } +// reconcileDelete deletes the Oxide resources owned by the OxideCluster and removes the +// finalizer. +func (r *OxideClusterReconciler) reconcileDelete( + ctx context.Context, + oxideClient cloud.OxideClient, + oxideCluster *infrav1.OxideCluster, + projectName string, + ipName string, +) error { + if err := r.ensureFloatingIPDeleted(ctx, oxideClient, projectName, ipName); err != nil { + return fmt.Errorf("deleting floating ip: %w", err) + } + // Delete the control plane anti-affinity group unconditionally rather than only when the + // policy is currently set, so a group created before the policy was unset is cleaned up too. + // CAPI deletes the cluster's Machines before its infrastructure, so the group has no members + // by the time this runs. + if err := r.ensureAntiAffinityGroupDeleted( + ctx, + oxideClient, + projectName, + getAntiAffinityGroupName(oxideCluster), + ); err != nil { + return fmt.Errorf("deleting anti-affinity group: %w", err) + } + controllerutil.RemoveFinalizer(oxideCluster, infrav1.ClusterFinalizer) + return nil +} + +// reconcileAntiAffinityGroup ensures the control plane anti-affinity group exists when the +// cluster opts in via ControlPlaneAntiAffinityPolicy, and is a no-op otherwise. +func (r *OxideClusterReconciler) reconcileAntiAffinityGroup( + ctx context.Context, + oxideClient cloud.OxideClient, + oxideCluster *infrav1.OxideCluster, + projectName string, +) error { + if oxideCluster.Spec.ControlPlaneAntiAffinityPolicy == "" { + return nil + } + _, err := r.ensureAntiAffinityGroupExists( + ctx, + oxideClient, + oxideCluster, + projectName, + getAntiAffinityGroupName(oxideCluster), + ) + return err +} + +// ensureAntiAffinityGroupExists creates or views the control plane anti-affinity group. An +// existing group is adopted as is: the Oxide API doesn't support updating a group's policy, so +// drift between the group and ControlPlaneAntiAffinityPolicy isn't reconciled. +func (r *OxideClusterReconciler) ensureAntiAffinityGroupExists( + ctx context.Context, + oxideClient cloud.OxideClient, + oxideCluster *infrav1.OxideCluster, + projectName string, + groupName string, +) (*oxide.AntiAffinityGroup, error) { + group, err := oxideClient.AntiAffinityGroupView(ctx, oxide.AntiAffinityGroupViewParams{ + Project: oxide.NameOrId(projectName), + AntiAffinityGroup: oxide.NameOrId(groupName), + }) + if err != nil { + if !errors.Is(err, oxide.ErrObjectNotFound) { + return nil, fmt.Errorf("fetching existing anti-affinity group: %w", err) + } + + group, err = oxideClient.AntiAffinityGroupCreate(ctx, oxide.AntiAffinityGroupCreateParams{ + Project: oxide.NameOrId(projectName), + Body: &oxide.AntiAffinityGroupCreate{ + Name: oxide.Name(groupName), + Description: fmt.Sprintf( + "Control plane anti-affinity for CAPI cluster %s/%s", + oxideCluster.Namespace, + oxideCluster.Name, + ), + FailureDomain: oxide.FailureDomainSled, + Policy: oxide.AffinityPolicy( + oxideCluster.Spec.ControlPlaneAntiAffinityPolicy, + ), + }, + }) + if err != nil { + return nil, fmt.Errorf("creating anti-affinity group: %w", err) + } + } + + return group, nil +} + +// ensureAntiAffinityGroupDeleted deletes the control plane anti-affinity group if it exists. +func (r *OxideClusterReconciler) ensureAntiAffinityGroupDeleted( + ctx context.Context, + oxideClient cloud.OxideClient, + projectName string, + groupName string, +) error { + if err := oxideClient.AntiAffinityGroupDelete(ctx, oxide.AntiAffinityGroupDeleteParams{ + Project: oxide.NameOrId(projectName), + AntiAffinityGroup: oxide.NameOrId(groupName), + }); err != nil { + if !errors.Is(err, oxide.ErrObjectNotFound) { + return err + } + } + return nil +} + // ensureFloatingIPDeleted deletes the floating IP if it exists. func (r *OxideClusterReconciler) ensureFloatingIPDeleted( ctx context.Context, diff --git a/internal/controller/oxidecluster_controller_test.go b/internal/controller/oxidecluster_controller_test.go index d7a79ab..ad15c03 100644 --- a/internal/controller/oxidecluster_controller_test.go +++ b/internal/controller/oxidecluster_controller_test.go @@ -153,6 +153,126 @@ func TestEnsureFloatingIPDeleted(t *testing.T) { } } +func TestEnsureAntiAffinityGroupExists(t *testing.T) { + wantGroup := &oxide.AntiAffinityGroup{ + Id: "group-id", + Name: "group-name", + } + + for _, tc := range []struct { + name string + setup func(*mock.MockOxideClient) + wantErr string + }{ + { + name: "create", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupView(gomock.Any(), gomock.Any()).Return(nil, httpErr("ObjectNotFound")) + m.EXPECT().AntiAffinityGroupCreate(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, params oxide.AntiAffinityGroupCreateParams) (*oxide.AntiAffinityGroup, error) { + assert.Equal(t, oxide.AffinityPolicyAllow, params.Body.Policy) + assert.Equal(t, oxide.FailureDomainSled, params.Body.FailureDomain) + return wantGroup, nil + }, + ) + }, + }, + { + name: "adopt", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupView(gomock.Any(), gomock.Any()).Return(wantGroup, nil) + }, + }, + { + name: "create error", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupView(gomock.Any(), gomock.Any()).Return(nil, httpErr("ObjectNotFound")) + m.EXPECT().AntiAffinityGroupCreate(gomock.Any(), gomock.Any()).Return(nil, httpErr("InternalError")) + }, + wantErr: "creating anti-affinity group", + }, + { + name: "view error", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupView(gomock.Any(), gomock.Any()).Return(nil, httpErr("InternalError")) + }, + wantErr: "fetching existing anti-affinity group", + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + oxideClient := mock.NewMockOxideClient(ctrl) + tc.setup(oxideClient) + + cluster := &infrav1.OxideCluster{ + Spec: infrav1.OxideClusterSpec{ControlPlaneAntiAffinityPolicy: "allow"}, + } + r := OxideClusterReconciler{} + gotGroup, gotErr := r.ensureAntiAffinityGroupExists( + context.Background(), + oxideClient, + cluster, + "project", + "group-name", + ) + if tc.wantErr != "" { + assert.ErrorContains(t, gotErr, tc.wantErr) + assert.Nil(t, gotGroup) + } else { + assert.NoError(t, gotErr) + assert.Equal(t, wantGroup, gotGroup) + } + }) + } +} + +func TestEnsureAntiAffinityGroupDeleted(t *testing.T) { + for _, tc := range []struct { + name string + setup func(*mock.MockOxideClient) + wantErr string + }{ + { + name: "delete", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupDelete(gomock.Any(), gomock.Any()).Return(nil) + }, + }, + { + name: "gone", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupDelete(gomock.Any(), gomock.Any()).Return(httpErr("ObjectNotFound")) + }, + }, + { + name: "delete error", + setup: func(m *mock.MockOxideClient) { + m.EXPECT().AntiAffinityGroupDelete(gomock.Any(), gomock.Any()).Return(httpErr("InternalError")) + }, + wantErr: "InternalError", + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + oxideClient := mock.NewMockOxideClient(ctrl) + tc.setup(oxideClient) + + r := OxideClusterReconciler{} + gotErr := r.ensureAntiAffinityGroupDeleted( + context.Background(), + oxideClient, + "project", + "group-name", + ) + if tc.wantErr != "" { + assert.ErrorContains(t, gotErr, tc.wantErr) + } else { + assert.NoError(t, gotErr) + } + }) + } +} + func TestFloatingIPAllocator(t *testing.T) { for _, tc := range []struct { name string diff --git a/internal/controller/oxidemachine_controller.go b/internal/controller/oxidemachine_controller.go index 3a93858..50beff4 100644 --- a/internal/controller/oxidemachine_controller.go +++ b/internal/controller/oxidemachine_controller.go @@ -21,6 +21,7 @@ import ( "encoding/base64" "errors" "fmt" + "slices" "strings" "time" @@ -162,13 +163,15 @@ func (r *OxideMachineReconciler) Reconcile( instance, err = oxideClient.InstanceCreate(ctx, oxide.InstanceCreateParams{ Project: oxide.NameOrId(projectName), Body: &oxide.InstanceCreate{ - Name: oxide.Name(instanceName), - Hostname: oxide.Hostname(instanceName), - Ncpus: oxide.InstanceCpuCount(oxideMachine.Spec.NCpus), - Memory: oxide.ByteCount(oxideMachine.Spec.Memory.Value()), - Start: new(true), - AntiAffinityGroups: toNamesOrIds(oxideMachine.Spec.AntiAffinityGroups), - SshPublicKeys: toNamesOrIds(oxideMachine.Spec.SSHPublicKeys), + Name: oxide.Name(instanceName), + Hostname: oxide.Hostname(instanceName), + Ncpus: oxide.InstanceCpuCount(oxideMachine.Spec.NCpus), + Memory: oxide.ByteCount(oxideMachine.Spec.Memory.Value()), + Start: new(true), + AntiAffinityGroups: toNamesOrIds( + instanceAntiAffinityGroups(oxideCluster, machine, oxideMachine), + ), + SshPublicKeys: toNamesOrIds(oxideMachine.Spec.SSHPublicKeys), UserData: base64.StdEncoding.EncodeToString( bootstrapSecret.Data["value"], ), @@ -499,6 +502,28 @@ func (r *OxideMachineReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +// instanceAntiAffinityGroups builds the anti-affinity group list for the machine's instance: the +// groups from the OxideMachine spec, plus the cluster-managed control plane group when the +// OxideCluster opts in via ControlPlaneAntiAffinityPolicy and the machine is a control plane +// machine. The OxideCluster reconciler guarantees the managed group exists before the cluster +// reports provisioned, i.e. before any Machine is created. +func instanceAntiAffinityGroups( + oxideCluster *infrav1.OxideCluster, + machine *clusterv1.Machine, + oxideMachine *infrav1.OxideMachine, +) []string { + groups := oxideMachine.Spec.AntiAffinityGroups + if oxideCluster.Spec.ControlPlaneAntiAffinityPolicy == "" || + !util.IsControlPlaneMachine(machine) { + return groups + } + groupName := getAntiAffinityGroupName(oxideCluster) + if slices.Contains(groups, groupName) { + return groups + } + return append(slices.Clone(groups), groupName) +} + func toNamesOrIds(values []string) []oxide.NameOrId { namesOrIds := make([]oxide.NameOrId, 0, len(values)) for _, value := range values { diff --git a/internal/controller/oxidemachine_controller_test.go b/internal/controller/oxidemachine_controller_test.go index a3bd058..e93495e 100644 --- a/internal/controller/oxidemachine_controller_test.go +++ b/internal/controller/oxidemachine_controller_test.go @@ -414,3 +414,76 @@ func TestDisksFromOxideMachine(t *testing.T) { } assert.Equal(t, got, want) } + +func TestInstanceAntiAffinityGroups(t *testing.T) { + oxideCluster := &infrav1.OxideCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: "default", + }, + Spec: infrav1.OxideClusterSpec{ControlPlaneAntiAffinityPolicy: "allow"}, + } + managedGroup := "capi-aag-default-cluster" + + controlPlaneMachine := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{clusterv1.MachineControlPlaneLabel: ""}, + }, + } + workerMachine := &clusterv1.Machine{} + + for _, tc := range []struct { + name string + oxideCluster *infrav1.OxideCluster + machine *clusterv1.Machine + specGroups []string + want []string + }{ + { + name: "control plane joins managed group", + oxideCluster: oxideCluster, + machine: controlPlaneMachine, + want: []string{managedGroup}, + }, + { + name: "spec groups are preserved", + oxideCluster: oxideCluster, + machine: controlPlaneMachine, + specGroups: []string{"user-group"}, + want: []string{"user-group", managedGroup}, + }, + { + name: "managed group not duplicated", + oxideCluster: oxideCluster, + machine: controlPlaneMachine, + specGroups: []string{managedGroup}, + want: []string{managedGroup}, + }, + { + name: "worker machine unaffected", + oxideCluster: oxideCluster, + machine: workerMachine, + specGroups: []string{"user-group"}, + want: []string{"user-group"}, + }, + { + name: "policy unset", + oxideCluster: &infrav1.OxideCluster{ + ObjectMeta: oxideCluster.ObjectMeta, + }, + machine: controlPlaneMachine, + specGroups: []string{"user-group"}, + want: []string{"user-group"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + oxideMachine := &infrav1.OxideMachine{ + Spec: infrav1.OxideMachineSpec{AntiAffinityGroups: tc.specGroups}, + } + got := instanceAntiAffinityGroups(tc.oxideCluster, tc.machine, oxideMachine) + assert.Equal(t, tc.want, got) + // The spec's group list must not be mutated by the append. + assert.Equal(t, tc.specGroups, oxideMachine.Spec.AntiAffinityGroups) + }) + } +} diff --git a/templates/cluster-template.yaml b/templates/cluster-template.yaml index 933bd5c..a45f36f 100644 --- a/templates/cluster-template.yaml +++ b/templates/cluster-template.yaml @@ -28,6 +28,7 @@ spec: vpc: "${OXIDE_VPC:-default}" subnet: "${OXIDE_SUBNET:-default}" ipPool: ${OXIDE_IP_POOL:-""} + controlPlaneAntiAffinityPolicy: allow --- apiVersion: controlplane.cluster.x-k8s.io/v1beta2 kind: KubeadmControlPlane