Skip to content
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.

Sign up for GitHub

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

Jump to bottom

✨ Add Support for FailureDomains #793

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

✨ Add Support for FailureDomains #793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2799759
Add FailureDomain to M3Cluster and Machine
sf1tzp Nov 30, 2022
8c69c14
Remove v1alpha5 generated code & lint
sf1tzp Feb 28, 2023
7c21ccd
Rebased & Generated
sf1tzp Sep 27, 2023
cd0cb80
Add conversion for m3m spec
sf1tzp Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/v1alpha5/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,11 @@ func Convert_v1beta1_FromPool_To_v1alpha5_FromPool(in *v1beta1.FromPool, out *Fr
return autoConvert_v1beta1_FromPool_To_v1alpha5_FromPool(in, out, s)
}

func Convert_v1beta1_Metal3MachineSpec_To_v1alpha5_Metal3MachineSpec(in *v1beta1.Metal3MachineSpec, out *Metal3MachineSpec, s apiconversion.Scope) error {
// ProviderID was added with v1beta1.
return autoConvert_v1beta1_Metal3MachineSpec_To_v1alpha5_Metal3MachineSpec(in, out, s)
}

func (src *Metal3DataTemplateList) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*v1beta1.Metal3DataTemplateList)
return Convert_v1alpha5_Metal3DataTemplateList_To_v1beta1_Metal3DataTemplateList(src, dst, nil)
Expand Down
41 changes: 29 additions & 12 deletions api/v1alpha5/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions api/v1beta1/metal3cluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ type Metal3ClusterStatus struct {
// +optional
FailureMessage *string `json:"failureMessage,omitempty"`

// FailureDomains specifies the failure domains available in the cluster.
// This will be used by Cluster API to try and spread the machines across failure domains
// +optional
FailureDomains clusterv1.FailureDomains `json:"failureDomains,omitempty"`

// Ready denotes that the Metal3 cluster (infrastructure) is ready. In
// Baremetal case, it does not mean anything for now as no infrastructure
// steps need to be performed. Required by Cluster API. Set to True by the
Expand Down
4 changes: 4 additions & 0 deletions api/v1beta1/metal3machine_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ type Metal3MachineSpec struct {
// +kubebuilder:validation:Enum:=metadata;disabled
// +optional
AutomatedCleaningMode *string `json:"automatedCleaningMode,omitempty"`

// FailureDomain is the failure domain the machine will be created in.
// +optional
FailureDomain *string `json:"failureDomain,omitempty"`
}

// Metal3MachineStatus defines the observed state of Metal3Machine.
Expand Down
12 changes: 12 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions baremetal/metal3machine_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ const (
ProviderIDPrefix = "metal3://"
// ProviderLabelPrefix is a label prefix for ProviderID.
ProviderLabelPrefix = "metal3.io/uuid"
// FailureDomainLabelPrefix is a label prefix for FailureDomains.
FailureDomainLabelPrefix = "infrastructure.cluster.x-k8s.io/failure-domain"
)

var (
Expand Down Expand Up @@ -831,6 +833,14 @@ func (m *MachineManager) chooseHost(ctx context.Context) (*bmov1alpha1.BareMetal
}
reqs = append(reqs, *r)
}
if m.Metal3Machine.Spec.FailureDomain != nil {
r, err := labels.NewRequirement(FailureDomainLabelPrefix, selection.Equals, []string{*m.Metal3Machine.Spec.FailureDomain})
if err != nil {
m.Log.Error(err, "Failed to create FailureDomain MatchLabel requirement, not choosing host")
return nil, nil, err
}
reqs = append(reqs, *r)
}
Comment on lines +836 to +843
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lentzi90 Sorry, I'm uninitated on conversion-gen. In the make generate output, I see

hack/tools/bin/conversion-gen \
                --input-dirs=./api/v1alpha5 \
                --output-file-base=zz_generated.conversion  --output-base=/Users/steven/oss/cluster-api-provider-metal3 \
                --go-header-file=./hack/boilerplate/boilerplate.generatego.txt
E0926 16:50:26.490736   38078 conversion.go:756] Warning: could not find nor generate a final Conversion function for github.com/metal3-io/cluster-api-provider-metal3/api/v1beta1.Metal3MachineSpec -> ./api/v1alpha5.Metal3MachineSpec
E0926 16:50:26.491121   38078 conversion.go:757]   the following fields need manual conversion:
E0926 16:50:26.491126   38078 conversion.go:759]       - FailureDomain

And in the generated conversion file

+++ b/api/v1alpha5/zz_generated.conversion.go
@@ -1246,6 +1246,7 @@ func autoConvert_v1beta1_Metal3ClusterStatus_To_v1alpha5_Metal3ClusterStatus(in
        out.LastUpdated = (*v1.Time)(unsafe.Pointer(in.LastUpdated))
        out.FailureReason = (*errors.ClusterStatusError)(unsafe.Pointer(in.FailureReason))
        out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage))
+       // WARNING: in.FailureDomains requires manual conversion: does not exist in peer-type
        out.Ready = in.Ready
        // WARNING: in.Conditions requires manual conversion: does not exist in peer-type
        return nil
@@ -1715,14 +1716,10 @@ func autoConvert_v1beta1_Metal3MachineSpec_To_v1alpha5_Metal3MachineSpec(in *v1b
        out.MetaData = (*corev1.SecretReference)(unsafe.Pointer(in.MetaData))
        out.NetworkData = (*corev1.SecretReference)(unsafe.Pointer(in.NetworkData))
        out.AutomatedCleaningMode = (*string)(unsafe.Pointer(in.AutomatedCleaningMode))
+       // WARNING: in.FailureDomain requires manual conversion: does not exist in peer-type
        return nil
 }

But I am not sure where to make this manual conversion. I read a bit into it, and found in our v1alpha5 doc.go

// +k8s:conversion-gen=github.com/metal3-io/cluster-api-provider-metal3/api/v1beta1

Which looks right... but do I need to add annotations to the M3Cluster and M3Machine types in v1alpha5 as well?

If you could help point me in the right direction, I would appreciate it! :) Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!
Since you are adding a new field here to v1beta1 that does not exist in v1alpha5, conversion-gen does not know what to do with it. We cannot really do anything about it, other than just dropping the field and "taking the responsibility" for it 😅 . We do that by creating a conversion function and then just calling the automatically generated function, similar to this:

cluster-api-provider-metal3/api/v1alpha5/conversion.go

Lines 219 to 232 in cde6c51

func Convert_v1beta1_NetworkDataIPv6_To_v1alpha5_NetworkDataIPv6(in *v1beta1.NetworkDataIPv6, out *NetworkDataIPv6, s apiconversion.Scope) error {
// fromPoolRef was added with v1beta1.
return autoConvert_v1beta1_NetworkDataIPv6_To_v1alpha5_NetworkDataIPv6(in, out, s)
}
func Convert_v1beta1_NetworkDataIPv4_To_v1alpha5_NetworkDataIPv4(in *v1beta1.NetworkDataIPv4, out *NetworkDataIPv4, s apiconversion.Scope) error {
// fromPoolRef was added with v1beta1.
return autoConvert_v1beta1_NetworkDataIPv4_To_v1alpha5_NetworkDataIPv4(in, out, s)
}
func Convert_v1beta1_FromPool_To_v1alpha5_FromPool(in *v1beta1.FromPool, out *FromPool, s apiconversion.Scope) error {
// apiGroup and kind was added with v1beta1.
return autoConvert_v1beta1_FromPool_To_v1alpha5_FromPool(in, out, s)
}

In other words, you need to add something like

func Convert_v1beta1_Metal3MachineSpec_To_v1alpha5_Metal3MachineSpec(in *v1beta1.Metal3MachineSpec, out *Metal3MachineSpec, s apiconversion.Scope) error {
	// FailureDomain was added in v1beta1 so there is nothing we can do to convert it.
	// It will simply be dropped when converting to v1alpha5.
	return autoConvert_v1beta1_Metal3MachineSpec_To_v1alpha5_Metal3MachineSpec(in, out, s)
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, thank you! I should have looked at this file, maybe I would have figured it out on my own. One more question though, why are we converting from v1beta1 to v1alpha5? Shouldn't these conversions be forward-looking?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually convert both ways. 🙂 This is a common pattern, and it works like this: There is one version that the controller handles, that is called the "hub" version. Then there can be any number of other supported versions called "spoken" versions. We add conversion functions to convert between the hub and the spoken versions (not between spoken versions directly, everything is always converted to/from the hub).

This system allows the controller to support any number of API versions simultaneously as long as we can convert them to/from the hub. The user can work with v1alpha5 as long as it is supported even though the controller is actually working with v1beta1. If the user asks for an object using v1alpha5, the webhook automatically converts it before returning it. Similarly, if the user creates an object using v1alpha5, it will be automatically converted and stored as v1beta1.

Kube-builder has some nice documentation around this if you want to read more: https://book.kubebuilder.io/multiversion-tutorial/conversion-concepts

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for explaining! And for the link. Cheers

labelSelector = labelSelector.Add(reqs...)

availableHosts := []*bmov1alpha1.BareMetalHost{}
Expand Down
21 changes: 21 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_metal3clusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,27 @@ spec:
- type
type: object
type: array
failureDomains:
additionalProperties:
description: FailureDomainSpec is the Schema for Cluster API failure
domains. It allows controllers to understand how many failure
domains a cluster can optionally span across.
properties:
attributes:
additionalProperties:
type: string
description: Attributes is a free form map of attributes an
infrastructure provider might use or require.
type: object
controlPlane:
description: ControlPlane determines if this failure domain
is suitable for use by control plane machines.
type: boolean
type: object
description: FailureDomains specifies the failure domains available
in the cluster. This will be used by Cluster API to try and spread
the machines across failure domains
type: object
failureMessage:
description: FailureMessage indicates that there is a fatal problem
reconciling the state, and will be set to a descriptive error message.
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_metal3machines.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,10 @@ spec:
type: string
type: object
x-kubernetes-map-type: atomic
failureDomain:
description: FailureDomain is the failure domain the machine will
be created in.
type: string
hostSelector:
description: HostSelector specifies matching criteria for labels on
BareMetalHosts. This is used to limit the set of BareMetalHost objects
Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_metal3machinetemplates.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ spec:
type: string
type: object
x-kubernetes-map-type: atomic
failureDomain:
description: FailureDomain is the failure domain the machine
will be created in.
type: string
hostSelector:
description: HostSelector specifies matching criteria for
labels on BareMetalHosts. This is used to limit the set
Expand Down