From 54f900e3e382bbdc8fb3bb65b97be1fc33546e5a Mon Sep 17 00:00:00 2001 From: Waleed Malik Date: Mon, 4 Mar 2024 15:00:36 +0300 Subject: [PATCH] Configure provider-id for the machines/nodes (#1723) (#1768) * Configure provider-id for the machines/nodes MC will inject provider-id for the machines that are created against cloud providers that don't have in-tree or external CCM support * Add exception in golangci-lint * Refactored code --------- Signed-off-by: Waleed Malik --- .golangci.yml | 1 + pkg/admission/machines.go | 10 +++++ pkg/cloudprovider/provider/anexia/instance.go | 3 ++ .../provider/digitalocean/provider.go | 3 ++ .../provider/equinixmetal/provider.go | 3 ++ pkg/cloudprovider/provider/gce/instance.go | 3 ++ .../provider/hetzner/provider.go | 3 ++ .../provider/kubevirt/provider.go | 3 ++ pkg/cloudprovider/provider/linode/provider.go | 3 ++ .../provider/nutanix/provider.go | 3 ++ .../provider/opennebula/provider.go | 3 ++ .../provider/openstack/provider.go | 3 ++ .../provider/vmwareclouddirector/provider.go | 3 ++ .../provider/vsphere/provider.go | 3 ++ pkg/cloudprovider/provider/vultr/provider.go | 3 ++ pkg/controller/machine/controller.go | 40 ++++++++++++++++++- pkg/providerconfig/types/types.go | 23 +++++++++++ 17 files changed, 111 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cc14d4190..7252ce8fa 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -59,3 +59,4 @@ issues: - 'cyclomatic complexity 34 of func `\(\*provider\)\.getConfig` is high' - 'cyclomatic complexity 31 of func `\(\*provider\)\.Validate` is high' - 'cyclomatic complexity 33 of func `\(\*provider\)\.Create` is high' + - 'cyclomatic complexity 32 of func `\(\*Reconciler\)\.ensureInstanceExistsForMachine` is high' diff --git a/pkg/admission/machines.go b/pkg/admission/machines.go index d96b9e977..2ee451bcb 100644 --- a/pkg/admission/machines.go +++ b/pkg/admission/machines.go @@ -64,6 +64,16 @@ func (ad *admissionData) mutateMachines(ctx context.Context, ar admissionv1.Admi if oldMachine.Spec.Name != machine.Spec.Name && machine.Spec.Name == machine.Name { oldMachine.Spec.Name = machine.Spec.Name } + + if oldMachine.Spec.ProviderID != nil && machine.Spec.ProviderID != nil && *oldMachine.Spec.ProviderID != *machine.Spec.ProviderID { + return nil, fmt.Errorf("providerID is immutable") + } + + // Allow mutation of the ProviderID field, as it can only be computed after the machine is created. + if oldMachine.Spec.ProviderID == nil && machine.Spec.ProviderID != nil { + oldMachine.Spec.ProviderID = machine.Spec.ProviderID + } + // Allow mutation when: // * machine has the `MigrationBypassSpecNoModificationRequirementAnnotation` annotation (used for type migration) bypassValidationForMigration := machine.Annotations[BypassSpecNoModificationRequirementAnnotation] == "true" diff --git a/pkg/cloudprovider/provider/anexia/instance.go b/pkg/cloudprovider/provider/anexia/instance.go index d84d90f7c..0c8343b1f 100644 --- a/pkg/cloudprovider/provider/anexia/instance.go +++ b/pkg/cloudprovider/provider/anexia/instance.go @@ -51,6 +51,9 @@ func (ai *anexiaInstance) ID() string { } func (ai *anexiaInstance) ProviderID() string { + if ai == nil || ai.ID() == "" { + return "" + } return ai.ID() } diff --git a/pkg/cloudprovider/provider/digitalocean/provider.go b/pkg/cloudprovider/provider/digitalocean/provider.go index d6a2c554a..12ce8be66 100644 --- a/pkg/cloudprovider/provider/digitalocean/provider.go +++ b/pkg/cloudprovider/provider/digitalocean/provider.go @@ -506,6 +506,9 @@ func (d *doInstance) ID() string { } func (d *doInstance) ProviderID() string { + if d.droplet == nil || d.droplet.Name == "" { + return "" + } return fmt.Sprintf("digitalocean://%d", d.droplet.ID) } diff --git a/pkg/cloudprovider/provider/equinixmetal/provider.go b/pkg/cloudprovider/provider/equinixmetal/provider.go index 2bbcb6aa5..645301fab 100644 --- a/pkg/cloudprovider/provider/equinixmetal/provider.go +++ b/pkg/cloudprovider/provider/equinixmetal/provider.go @@ -400,6 +400,9 @@ func (s *metalDevice) ID() string { } func (s *metalDevice) ProviderID() string { + if s.device == nil || s.device.ID == "" { + return "" + } return "equinixmetal://" + s.device.ID } diff --git a/pkg/cloudprovider/provider/gce/instance.go b/pkg/cloudprovider/provider/gce/instance.go index 1d61d4bae..2b6476195 100644 --- a/pkg/cloudprovider/provider/gce/instance.go +++ b/pkg/cloudprovider/provider/gce/instance.go @@ -61,6 +61,9 @@ func (gi *googleInstance) ID() string { } func (gi *googleInstance) ProviderID() string { + if gi.ci == nil || gi.ci.Name == "" { + return "" + } return fmt.Sprintf("gce://%s/%s/%s", gi.projectID, gi.zone, gi.ci.Name) } diff --git a/pkg/cloudprovider/provider/hetzner/provider.go b/pkg/cloudprovider/provider/hetzner/provider.go index 83521dccf..8f1a88b4a 100644 --- a/pkg/cloudprovider/provider/hetzner/provider.go +++ b/pkg/cloudprovider/provider/hetzner/provider.go @@ -553,6 +553,9 @@ func (s *hetznerServer) ID() string { } func (s *hetznerServer) ProviderID() string { + if s.server == nil || s.server.ID == 0 { + return "" + } return fmt.Sprintf("hcloud://%d", s.server.ID) } diff --git a/pkg/cloudprovider/provider/kubevirt/provider.go b/pkg/cloudprovider/provider/kubevirt/provider.go index a2ba1f58c..2102f84bf 100644 --- a/pkg/cloudprovider/provider/kubevirt/provider.go +++ b/pkg/cloudprovider/provider/kubevirt/provider.go @@ -160,6 +160,9 @@ func (k *kubeVirtServer) ID() string { } func (k *kubeVirtServer) ProviderID() string { + if k.vmi.Name == "" { + return "" + } return "kubevirt://" + k.vmi.Name } diff --git a/pkg/cloudprovider/provider/linode/provider.go b/pkg/cloudprovider/provider/linode/provider.go index c3ef9ebe3..0af5bc186 100644 --- a/pkg/cloudprovider/provider/linode/provider.go +++ b/pkg/cloudprovider/provider/linode/provider.go @@ -405,6 +405,9 @@ func (d *linodeInstance) ID() string { } func (d *linodeInstance) ProviderID() string { + if d == nil || d.ID() == "" { + return "" + } return fmt.Sprintf("linode://%s", d.ID()) } diff --git a/pkg/cloudprovider/provider/nutanix/provider.go b/pkg/cloudprovider/provider/nutanix/provider.go index c93816d58..86e192503 100644 --- a/pkg/cloudprovider/provider/nutanix/provider.go +++ b/pkg/cloudprovider/provider/nutanix/provider.go @@ -89,6 +89,9 @@ func (nutanixServer Server) ID() string { } func (nutanixServer Server) ProviderID() string { + if nutanixServer.ID() == "" { + return "" + } return fmt.Sprintf("nutanix://%s", nutanixServer.ID()) } diff --git a/pkg/cloudprovider/provider/opennebula/provider.go b/pkg/cloudprovider/provider/opennebula/provider.go index 1108bb8b6..e73d329ed 100644 --- a/pkg/cloudprovider/provider/opennebula/provider.go +++ b/pkg/cloudprovider/provider/opennebula/provider.go @@ -427,6 +427,9 @@ func (i *openNebulaInstance) ID() string { } func (i *openNebulaInstance) ProviderID() string { + if i.vm == nil || i.vm.ID == 0 { + return "" + } return "opennebula://" + strconv.Itoa(i.vm.ID) } diff --git a/pkg/cloudprovider/provider/openstack/provider.go b/pkg/cloudprovider/provider/openstack/provider.go index 65a6096dc..5fab7d65f 100644 --- a/pkg/cloudprovider/provider/openstack/provider.go +++ b/pkg/cloudprovider/provider/openstack/provider.go @@ -936,6 +936,9 @@ func (d *osInstance) ID() string { } func (d *osInstance) ProviderID() string { + if d.server == nil || d.server.ID == "" { + return "" + } return "openstack:///" + d.server.ID } diff --git a/pkg/cloudprovider/provider/vmwareclouddirector/provider.go b/pkg/cloudprovider/provider/vmwareclouddirector/provider.go index c67169efc..7092ec7eb 100644 --- a/pkg/cloudprovider/provider/vmwareclouddirector/provider.go +++ b/pkg/cloudprovider/provider/vmwareclouddirector/provider.go @@ -123,6 +123,9 @@ func (s Server) ID() string { } func (s Server) ProviderID() string { + if s.ID() == "" { + return "" + } return fmt.Sprintf("vmware-cloud-director://%s", s.ID()) } diff --git a/pkg/cloudprovider/provider/vsphere/provider.go b/pkg/cloudprovider/provider/vsphere/provider.go index 93f9bf552..f27cba0df 100644 --- a/pkg/cloudprovider/provider/vsphere/provider.go +++ b/pkg/cloudprovider/provider/vsphere/provider.go @@ -101,6 +101,9 @@ func (vsphereServer Server) ID() string { } func (vsphereServer Server) ProviderID() string { + if vsphereServer.uuid == "" { + return "" + } return "vsphere://" + vsphereServer.uuid } diff --git a/pkg/cloudprovider/provider/vultr/provider.go b/pkg/cloudprovider/provider/vultr/provider.go index 3f4e63d5a..b8ee1d9e0 100644 --- a/pkg/cloudprovider/provider/vultr/provider.go +++ b/pkg/cloudprovider/provider/vultr/provider.go @@ -338,6 +338,9 @@ func (v *vultrInstance) ID() string { } func (v *vultrInstance) ProviderID() string { + if v.instance == nil || v.instance.ID == "" { + return "" + } return "vultr://" + v.instance.ID } diff --git a/pkg/controller/machine/controller.go b/pkg/controller/machine/controller.go index d952ec113..2e730a67c 100644 --- a/pkg/controller/machine/controller.go +++ b/pkg/controller/machine/controller.go @@ -100,6 +100,9 @@ const ( // AnnotationAutoscalerIdentifier is used by the cluster-autoscaler // cluster-api provider to match Nodes to Machines. AnnotationAutoscalerIdentifier = "cluster.k8s.io/machine" + + // ProviderID pattern. + ProviderIDPattern = "kubermatic://%s/%s" ) // Reconciler is the controller implementation for machine resources. @@ -478,7 +481,24 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, mach return r.ensureInstanceExistsForMachine(ctx, log, prov, machine, userdataPlugin, providerConfig) } - // case 3.3: if the node exists make sure if it has labels and taints attached to it. + // case 3.3: if the node exists and both external and internal CCM are not available. Then set the provider-id for the node. + inTree, err := providerconfigtypes.IntreeCloudProviderImplementationSupported(providerConfig.CloudProvider, machine.Spec.Versions.Kubelet) + if err != nil { + return nil, fmt.Errorf("failed to check if cloud provider %q has in-tree implementation: %w", providerConfig.CloudProvider, err) + } + + if !inTree && !r.nodeSettings.ExternalCloudProvider && node.Spec.ProviderID == "" { + providerID := fmt.Sprintf(ProviderIDPattern, providerConfig.CloudProvider, machine.UID) + if err := r.updateNode(ctx, node, func(n *corev1.Node) { + n.Spec.ProviderID = providerID + }); err != nil { + return nil, fmt.Errorf("failed to update node %s with the ProviderID: %w", node.Name, err) + } + + r.recorder.Event(machine, corev1.EventTypeNormal, "ProviderIDUpdated", "Successfully updated providerID on node") + nodeLog.Info("Added ProviderID to the node") + } + // case 3.4: if the node exists make sure if it has labels and taints attached to it. return nil, r.ensureNodeLabelsAnnotationsAndTaints(ctx, nodeLog, node, machine) } @@ -978,10 +998,26 @@ func (r *Reconciler) ensureInstanceExistsForMachine( return a.Type < b.Type }) + var providerID string + if machine.Spec.ProviderID == nil { + inTree, err := providerconfigtypes.IntreeCloudProviderImplementationSupported(providerConfig.CloudProvider, machine.Spec.Versions.Kubelet) + if err != nil { + return nil, fmt.Errorf("failed to check if cloud provider %q has in-tree implementation: %w", providerConfig.CloudProvider, err) + } + + // If both external and internal CCM are not available. We set provider-id for the machine explicitly. + if !inTree && !r.nodeSettings.ExternalCloudProvider { + providerID = fmt.Sprintf(ProviderIDPattern, providerConfig.CloudProvider, machine.UID) + } + } + if err := r.updateMachine(machine, func(m *clusterv1alpha1.Machine) { m.Status.Addresses = machineAddresses + if providerID != "" { + m.Spec.ProviderID = &providerID + } }); err != nil { - return nil, fmt.Errorf("failed to update machine after setting .status.addresses: %w", err) + return nil, fmt.Errorf("failed to update machine after setting .status.addresses and providerID: %w", err) } return r.ensureNodeOwnerRef(ctx, log, providerInstance, machine, providerConfig) diff --git a/pkg/providerconfig/types/types.go b/pkg/providerconfig/types/types.go index 18bd2010d..046f4cd9f 100644 --- a/pkg/providerconfig/types/types.go +++ b/pkg/providerconfig/types/types.go @@ -23,6 +23,8 @@ import ( "fmt" "strconv" + "github.com/Masterminds/semver/v3" + clusterv1alpha1 "github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1" "github.com/kubermatic/machine-controller/pkg/cloudprovider/util" "github.com/kubermatic/machine-controller/pkg/jsonutil" @@ -107,6 +109,27 @@ var ( } ) +func IntreeCloudProviderImplementationSupported(cloudProvider CloudProvider, version string) (inTree bool, err error) { + kubeletVer, err := semver.NewVersion(version) + if err != nil { + return false, fmt.Errorf("failed to parse kubelet version: %w", err) + } + + switch cloudProvider { + case CloudProviderAzure, CloudProviderVsphere, CloudProviderGoogle: + return true, nil + case CloudProviderAWS: + // In-tree AWS support was removed in Kubernetes 1.27. + ltKube127Condition, _ := semver.NewConstraint("< 1.27") + if ltKube127Condition.Check(kubeletVer) { + return true, nil + } + return false, nil + default: + return false, nil + } +} + // DNSConfig contains a machine's DNS configuration. type DNSConfig struct { Servers []string `json:"servers"`