diff --git a/.gitignore b/.gitignore index fa257cb80..7f43b125e 100644 --- a/.gitignore +++ b/.gitignore @@ -83,3 +83,5 @@ watchall-output* /conditions*.txt /.config + +/.kube diff --git a/Makefile b/Makefile index 72887dc58..c6dff3609 100644 --- a/Makefile +++ b/Makefile @@ -192,18 +192,21 @@ install-cilium-in-wl-cluster: -f templates/cilium/cilium.yaml -install-ccm-in-wl-cluster: +install-ccm-in-wl-cluster: $(WORKER_CLUSTER_KUBECONFIG) ifeq ($(BUILD_IN_CONTAINER),true) docker run --rm \ + -e CLUSTER_NAME=$(CLUSTER_NAME) \ + -e KUBECONFIG=$(KUBECONFIG) \ -v $(shell go env GOPATH)/pkg:/go/pkg$(MOUNT_FLAGS) \ -v $(shell pwd):/src/cluster-api-provider-$(INFRA_PROVIDER)$(MOUNT_FLAGS) \ $(BUILDER_IMAGE):$(BUILDER_IMAGE_VERSION) $@; else - helm repo add syself https://charts.syself.com - helm repo update syself - KUBECONFIG=$(WORKER_CLUSTER_KUBECONFIG) helm upgrade --install ccm syself/ccm-hetzner --version 2.0.1 \ - --namespace kube-system \ - --set privateNetwork.enabled=$(PRIVATE_NETWORK) + helm repo add hcloud https://charts.hetzner.cloud + helm repo update hcloud + KUBECONFIG=$(WORKER_CLUSTER_KUBECONFIG) helm install hccm \ + hcloud/hcloud-cloud-controller-manager -n kube-system \ + --set privateNetwork.enabled=$(PRIVATE_NETWORK) \ + --set robot.enabled=true @echo 'run "kubectl --kubeconfig=$(WORKER_CLUSTER_KUBECONFIG) ..." to work with the new target cluster' endif @@ -437,6 +440,7 @@ $(ARTIFACTS): $(MGT_CLUSTER_KUBECONFIG): ./hack/get-kubeconfig-of-management-cluster.sh +.PHONY: $(WORKER_CLUSTER_KUBECONFIG) $(WORKER_CLUSTER_KUBECONFIG): ./hack/get-kubeconfig-of-workload-cluster.sh diff --git a/api/v1beta1/hetznerbaremetalmachine_types.go b/api/v1beta1/hetznerbaremetalmachine_types.go index a671c9806..fa110427c 100644 --- a/api/v1beta1/hetznerbaremetalmachine_types.go +++ b/api/v1beta1/hetznerbaremetalmachine_types.go @@ -67,8 +67,12 @@ const ( // HetznerBareMetalMachineSpec defines the desired state of HetznerBareMetalMachine. type HetznerBareMetalMachineSpec struct { - // ProviderID will be the hetznerbaremetalmachine which is set by the controller - // in the `hcloud://bm-` format. + // ProviderID is set by the controller in the `hrobot://` format. Before CAPH v1.1.0 + // the ProviderID had the format `hcloud://bm-NNNNN`. Starting with CAPH v1.1.0 this was changed + // to `hrobot://NNNNN`. This aligns with the upstream hcloud CCM. In the long run we want to + // discontinue our CCM fork. After the ProviderID was set once, it never changes. It is safe to + // update the CCM in a running workload-cluster. Only new nodes will get the new format. + // // +optional ProviderID *string `json:"providerID,omitempty"` diff --git a/api/v1beta1/hetznerbaremetalmachine_validation.go b/api/v1beta1/hetznerbaremetalmachine_validation.go index d27c3d6bd..b98c489a7 100644 --- a/api/v1beta1/hetznerbaremetalmachine_validation.go +++ b/api/v1beta1/hetznerbaremetalmachine_validation.go @@ -86,5 +86,14 @@ func validateHetznerBareMetalMachineSpecUpdate(oldSpec, newSpec HetznerBareMetal ) } + if oldSpec.ProviderID != nil && *oldSpec.ProviderID != "" { + // once the Provider was set, the value must not change. + if newSpec.ProviderID == nil || *oldSpec.ProviderID != *newSpec.ProviderID { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "providerID"), newSpec.ProviderID, "providerID immutable"), + ) + } + } + return allErrs } diff --git a/api/v1beta1/hetznerbaremetalmachine_validation_test.go b/api/v1beta1/hetznerbaremetalmachine_validation_test.go index 9c145031a..1e8844868 100644 --- a/api/v1beta1/hetznerbaremetalmachine_validation_test.go +++ b/api/v1beta1/hetznerbaremetalmachine_validation_test.go @@ -17,11 +17,14 @@ limitations under the License. package v1beta1 import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" ) func TestValidateHetznerBareMetalMachineSpecCreate(t *testing.T) { @@ -429,3 +432,43 @@ func TestValidateHetznerBareMetalMachineSpecUpdate(t *testing.T) { }) } } + +func TestValidateHetznerBareMetalMachineSpecUpdate_ProviderID(t *testing.T) { + got := validateHetznerBareMetalMachineSpecUpdate( + HetznerBareMetalMachineSpec{ + ProviderID: ptr.To("provider://foo"), + }, + HetznerBareMetalMachineSpec{}) + require.Equal(t, `[spec.providerID: Invalid value: "null": providerID immutable]`, fmt.Sprintf("%+v", got)) + + got = validateHetznerBareMetalMachineSpecUpdate( + HetznerBareMetalMachineSpec{ + ProviderID: ptr.To("provider://foo"), + }, + HetznerBareMetalMachineSpec{ + ProviderID: ptr.To("provider://bar"), + }) + require.Equal(t, `[spec.providerID: Invalid value: "provider://bar": providerID immutable]`, fmt.Sprintf("%+v", got)) + + // Allowed Updates + got = validateHetznerBareMetalMachineSpecUpdate( + HetznerBareMetalMachineSpec{}, + HetznerBareMetalMachineSpec{}) + require.Equal(t, `[]`, fmt.Sprintf("%+v", got)) + + got = validateHetznerBareMetalMachineSpecUpdate( + HetznerBareMetalMachineSpec{}, + HetznerBareMetalMachineSpec{ + ProviderID: ptr.To("provider://bar"), + }) + require.Equal(t, `[]`, fmt.Sprintf("%+v", got)) + + got = validateHetznerBareMetalMachineSpecUpdate( + HetznerBareMetalMachineSpec{ + ProviderID: ptr.To("provider://bar"), + }, + HetznerBareMetalMachineSpec{ + ProviderID: ptr.To("provider://bar"), + }) + require.Equal(t, `[]`, fmt.Sprintf("%+v", got)) +} diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index b08c598a2..18cdb0ec3 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -101,9 +101,19 @@ type HCloudPlacementGroupStatus struct { // HetznerSecretRef defines all the names of the secret and the relevant keys needed to access Hetzner API. type HetznerSecretRef struct { - // Name defines the name of the secret. - // +kubebuilder:default=hetzner + // Name defines the name of the secret. The name gets used for reading the credential in the + // mgt-cluster, and it gets used for creating a secret in the wl-cluster. About the secret in + // the wl-cluster: Attention, the upstream hcloud-ccm helm chart expects the name to be + // "hcloud". The Syself ccm defaults to "hetzner". For compatibility with upstream hcloud-ccm + // the controller creates two secrets, if the name is different from "hcloud" (one with name + // "hcloud", one with name being the value of this setting). The secret will be created in the + // namespace "kube-system" of the workload-cluster. We recommend to use "hcloud", because this is + // the default of upstream hcloud-ccm. Set `spec.skipCreatingHetznerSecretInWorkloadCluster`, if + // you don't want that secret in the wl-cluster to be created. + // + // +kubebuilder:default=hcloud Name string `json:"name"` + // Key defines the keys that are used in the secret. // Need to specify either HCloudToken or both HetznerRobotUser and HetznerRobotPassword. Key HetznerSecretKeyRef `json:"key"` @@ -112,19 +122,33 @@ type HetznerSecretRef struct { // HetznerSecretKeyRef defines the key name of the HetznerSecret. // Need to specify either HCloudToken or both HetznerRobotUser and HetznerRobotPassword. type HetznerSecretKeyRef struct { - // HCloudToken defines the name of the key where the token for the Hetzner Cloud API is stored. + // hcloudToken defines the name of the key where the token for the Hetzner Cloud API is stored. + // We recommend to use "token", because this is the default of upstream hcloud-ccm, while the + // legacy Syself ccm uses "hcloud". For maximal compatibility up to three keys get created in the + // secret for HCLOUD_TOKEN: "hcloud", "token" and the value of hcloudToken. This way we ensure + // that the ccm in the wl-cluster finds the secret. + // // +optional - // +kubebuilder:default=hcloud-token + // +kubebuilder:default=token HCloudToken string `json:"hcloudToken"` - // HetznerRobotUser defines the name of the key where the username for the Hetzner Robot API is stored. + + // HetznerRobotUser defines the name of the key where the username for the Hetzner Robot API is + // stored. We recommend to use "robot-user", because this is the default of upstream hcloud-ccm. + // // +optional - // +kubebuilder:default=hetzner-robot-user + // +kubebuilder:default=robot-user HetznerRobotUser string `json:"hetznerRobotUser"` - // HetznerRobotPassword defines the name of the key where the password for the Hetzner Robot API is stored. + + // HetznerRobotPassword defines the name of the key where the password for the Hetzner Robot API + // is stored. We recommend to use "robot-password", because this is the default of upstream + // hcloud-ccm. + // // +optional - // +kubebuilder:default=hetzner-robot-password + // +kubebuilder:default=robot-password HetznerRobotPassword string `json:"hetznerRobotPassword"` + // SSHKey defines the name of the ssh key. + // // +optional // +kubebuilder:default=hcloud-ssh-key-name SSHKey string `json:"sshKey"` diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml index 9d7cfe261..c319162fe 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachines.yaml @@ -254,8 +254,11 @@ spec: type: object providerID: description: |- - ProviderID will be the hetznerbaremetalmachine which is set by the controller - in the `hcloud://bm-` format. + ProviderID is set by the controller in the `hrobot://` format. Before CAPH v1.1.0 + the ProviderID had the format `hcloud://bm-NNNNN`. Starting with CAPH v1.1.0 this was changed + to `hrobot://NNNNN`. This aligns with the upstream hcloud CCM. In the long run we want to + discontinue our CCM fork. After the ProviderID was set once, it never changes. It is safe to + update the CCM in a running workload-cluster. Only new nodes will get the new format. type: string sshSpec: description: SSHSpec gives a reference on the secret where SSH details diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachinetemplates.yaml index c00a8329f..acee1141b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerbaremetalmachinetemplates.yaml @@ -241,8 +241,11 @@ spec: type: object providerID: description: |- - ProviderID will be the hetznerbaremetalmachine which is set by the controller - in the `hcloud://bm-` format. + ProviderID is set by the controller in the `hrobot://` format. Before CAPH v1.1.0 + the ProviderID had the format `hcloud://bm-NNNNN`. Starting with CAPH v1.1.0 this was changed + to `hrobot://NNNNN`. This aligns with the upstream hcloud CCM. In the long run we want to + discontinue our CCM fork. After the ProviderID was set once, it never changes. It is safe to + update the CCM in a running workload-cluster. Only new nodes will get the new format. type: string sshSpec: description: SSHSpec gives a reference on the secret where diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml index 852ed38bb..9c0d6dc30 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclusters.yaml @@ -246,19 +246,26 @@ spec: Need to specify either HCloudToken or both HetznerRobotUser and HetznerRobotPassword. properties: hcloudToken: - default: hcloud-token - description: HCloudToken defines the name of the key where - the token for the Hetzner Cloud API is stored. + default: token + description: |- + hcloudToken defines the name of the key where the token for the Hetzner Cloud API is stored. + We recommend to use "token", because this is the default of upstream hcloud-ccm, while the + legacy Syself ccm uses "hcloud". For maximal compatibility up to three keys get created in the + secret for HCLOUD_TOKEN: "hcloud", "token" and the value of hcloudToken. This way we ensure + that the ccm in the wl-cluster finds the secret. type: string hetznerRobotPassword: - default: hetzner-robot-password - description: HetznerRobotPassword defines the name of the - key where the password for the Hetzner Robot API is stored. + default: robot-password + description: |- + HetznerRobotPassword defines the name of the key where the password for the Hetzner Robot API + is stored. We recommend to use "robot-password", because this is the default of upstream + hcloud-ccm. type: string hetznerRobotUser: - default: hetzner-robot-user - description: HetznerRobotUser defines the name of the key - where the username for the Hetzner Robot API is stored. + default: robot-user + description: |- + HetznerRobotUser defines the name of the key where the username for the Hetzner Robot API is + stored. We recommend to use "robot-user", because this is the default of upstream hcloud-ccm. type: string sshKey: default: hcloud-ssh-key-name @@ -266,8 +273,17 @@ spec: type: string type: object name: - default: hetzner - description: Name defines the name of the secret. + default: hcloud + description: |- + Name defines the name of the secret. The name gets used for reading the credential in the + mgt-cluster, and it gets used for creating a secret in the wl-cluster. About the secret in + the wl-cluster: Attention, the upstream hcloud-ccm helm chart expects the name to be + "hcloud". The Syself ccm defaults to "hetzner". For compatibility with upstream hcloud-ccm + the controller creates two secrets, if the name is different from "hcloud" (one with name + "hcloud", one with name being the value of this setting). The secret will be created in the + namespace "kube-system" of the workload-cluster. We recommend to use "hcloud", because this is + the default of upstream hcloud-ccm. Set `spec.skipCreatingHetznerSecretInWorkloadCluster`, if + you don't want that secret in the wl-cluster to be created. type: string required: - key diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml index a4c1b20c7..b53b59aa3 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hetznerclustertemplates.yaml @@ -275,21 +275,26 @@ spec: Need to specify either HCloudToken or both HetznerRobotUser and HetznerRobotPassword. properties: hcloudToken: - default: hcloud-token - description: HCloudToken defines the name of the key - where the token for the Hetzner Cloud API is stored. + default: token + description: |- + hcloudToken defines the name of the key where the token for the Hetzner Cloud API is stored. + We recommend to use "token", because this is the default of upstream hcloud-ccm, while the + legacy Syself ccm uses "hcloud". For maximal compatibility up to three keys get created in the + secret for HCLOUD_TOKEN: "hcloud", "token" and the value of hcloudToken. This way we ensure + that the ccm in the wl-cluster finds the secret. type: string hetznerRobotPassword: - default: hetzner-robot-password - description: HetznerRobotPassword defines the name - of the key where the password for the Hetzner Robot - API is stored. + default: robot-password + description: |- + HetznerRobotPassword defines the name of the key where the password for the Hetzner Robot API + is stored. We recommend to use "robot-password", because this is the default of upstream + hcloud-ccm. type: string hetznerRobotUser: - default: hetzner-robot-user - description: HetznerRobotUser defines the name of - the key where the username for the Hetzner Robot - API is stored. + default: robot-user + description: |- + HetznerRobotUser defines the name of the key where the username for the Hetzner Robot API is + stored. We recommend to use "robot-user", because this is the default of upstream hcloud-ccm. type: string sshKey: default: hcloud-ssh-key-name @@ -297,8 +302,17 @@ spec: type: string type: object name: - default: hetzner - description: Name defines the name of the secret. + default: hcloud + description: |- + Name defines the name of the secret. The name gets used for reading the credential in the + mgt-cluster, and it gets used for creating a secret in the wl-cluster. About the secret in + the wl-cluster: Attention, the upstream hcloud-ccm helm chart expects the name to be + "hcloud". The Syself ccm defaults to "hetzner". For compatibility with upstream hcloud-ccm + the controller creates two secrets, if the name is different from "hcloud" (one with name + "hcloud", one with name being the value of this setting). The secret will be created in the + namespace "kube-system" of the workload-cluster. We recommend to use "hcloud", because this is + the default of upstream hcloud-ccm. Set `spec.skipCreatingHetznerSecretInWorkloadCluster`, if + you don't want that secret in the wl-cluster to be created. type: string required: - key diff --git a/controllers/csr_controller.go b/controllers/csr_controller.go index 5e5cfd726..1554af7be 100644 --- a/controllers/csr_controller.go +++ b/controllers/csr_controller.go @@ -325,7 +325,8 @@ func getHbmmWithConstantHostname(ctx context.Context, csrUsername string, cluste log := ctrl.LoggerFrom(ctx) clusterFromCSR, serverID := getServerIDFromConstantHostname(ctx, csrUsername, clusterName) - providerID := "hcloud://bm-" + serverID + legacyProviderID := "hcloud://bm-" + serverID + providerID := "hrobot://" + serverID hList := &infrav1.HetznerBareMetalMachineList{} selector := labels.NewSelector() req, err := labels.NewRequirement(clusterv1.ClusterNameLabel, selection.Equals, []string{clusterFromCSR}) @@ -348,13 +349,15 @@ func getHbmmWithConstantHostname(ctx context.Context, csrUsername string, cluste if hList.Items[i].Spec.ProviderID == nil { continue } - if *hList.Items[i].Spec.ProviderID == providerID { + if *hList.Items[i].Spec.ProviderID == providerID || + *hList.Items[i].Spec.ProviderID == legacyProviderID { hbmm = &hList.Items[i] break } } + if hbmm == nil { - return nil, fmt.Errorf("ProviderID: %q %w", providerID, errNoHetznerBareMetalMachineByProviderIDFound) + return nil, fmt.Errorf("ProviderID: %q (or %q): %w", providerID, legacyProviderID, errNoHetznerBareMetalMachineByProviderIDFound) } log.Info("Found HetznerBareMetalMachine with constant hostname", "csr-username", csrUsername, "hetznerBareMetalMachine", hbmm.Name) diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go index a66f971fb..e428d17b8 100644 --- a/controllers/hetznercluster_controller.go +++ b/controllers/hetznercluster_controller.go @@ -226,7 +226,7 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS // target cluster is ready conditions.MarkTrue(hetznerCluster, infrav1.TargetClusterReadyCondition) - result, err = reconcileTargetSecret(ctx, clusterScope) + result, err = reconcileWorkloadClusterSecrets(ctx, clusterScope) if err != nil { reterr := fmt.Errorf("failed to reconcile target secret: %w", err) conditions.MarkFalse( @@ -472,7 +472,12 @@ func hcloudTokenErrorResult( return res, nil } -func reconcileTargetSecret(ctx context.Context, clusterScope *scope.ClusterScope) (res reconcile.Result, reterr error) { +// reconcileWorkloadClusterSecrets ensures that the workload-cluster has the secret needed by the +// ccm. The name of the secret is read from HetznerCluster.Spec.HetznerSecret.Name. If +// HetznerSecret.Name is "hcloud", then only one secret gets created in the wl-cluster. If not, two +// secrets are created in the wl-cluster. This ensures compatibility between CCMs. Creating the +// secret gets skipped, if HetznerCluster.Spec.SkipCreatingHetznerSecretInWorkloadCluster is set. +func reconcileWorkloadClusterSecrets(ctx context.Context, clusterScope *scope.ClusterScope) (res reconcile.Result, reterr error) { if clusterScope.HetznerCluster.Spec.SkipCreatingHetznerSecretInWorkloadCluster { // If the secret should not be created in the workload cluster, we just return. // This means the ccm is running outside of the workload cluster (or getting the secret differently). @@ -480,13 +485,13 @@ func reconcileTargetSecret(ctx context.Context, clusterScope *scope.ClusterScope } // Checking if control plane is ready - clientConfig, err := clusterScope.ClientConfig(ctx) + wlClientConfig, err := clusterScope.ClientConfig(ctx) if err != nil { clusterScope.V(1).Info("failed to get clientconfig with api endpoint") return reconcile.Result{}, err } - if err := scope.IsControlPlaneReady(ctx, clientConfig); err != nil { + if err := scope.IsControlPlaneReady(ctx, wlClientConfig); err != nil { conditions.MarkFalse( clusterScope.HetznerCluster, infrav1.TargetClusterSecretReadyCondition, @@ -500,73 +505,118 @@ func reconcileTargetSecret(ctx context.Context, clusterScope *scope.ClusterScope // Control plane ready, so we can check if the secret exists already // getting client - restConfig, err := clientConfig.ClientConfig() + restConfig, err := wlClientConfig.ClientConfig() if err != nil { return reconcile.Result{}, fmt.Errorf("failed to get rest config: %w", err) } - client, err := client.New(restConfig, client.Options{}) + // workload cluster client + wlClient, err := client.New(restConfig, client.Options{}) if err != nil { return reconcile.Result{}, fmt.Errorf("failed to get client: %w", err) } - secret := &corev1.Secret{ + // To ensure compatibility with both CCMs, create always a secret with name "hcloud" in the + // wl-cluster. + names := []string{clusterScope.HetznerCluster.Spec.HetznerSecret.Name} + if clusterScope.HetznerCluster.Spec.HetznerSecret.Name != "hcloud" { + names = append(names, "hcloud") + } + + for _, name := range names { + err = reconcileOneWorkloadClusterSecret(ctx, clusterScope, wlClient, name) + if err != nil { + return reconcile.Result{}, fmt.Errorf("failed to reconcile wl-cluster secret %q: %w", + name, err) + } + } + return reconcile.Result{}, nil +} + +// reconcileOneWorkloadClusterSecret creates/updates the secret in the wl-cluster. For maximal +// compatibility up to three keys get created in the secret for HCLOUD_TOKEN: "hcloud", "token" and +// the value of HetznerCluster.Spec.HetznerSecret.Key.HCloudToken. See docstring of +// HetznerCluster.Spec.HetznerSecret.Key.HCloudToken. +func reconcileOneWorkloadClusterSecret(ctx context.Context, clusterScope *scope.ClusterScope, wlClient client.Client, name string) error { + wlSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: clusterScope.HetznerCluster.Spec.HetznerSecret.Name, + Name: name, Namespace: metav1.NamespaceSystem, }, } // Make sure secret exists and has the expected values - _, err = controllerutil.CreateOrUpdate(ctx, client, secret, func() error { - tokenSecretName := types.NamespacedName{ + _, err := controllerutil.CreateOrUpdate(ctx, wlClient, wlSecret, func() error { + mgtSecretName := types.NamespacedName{ Namespace: clusterScope.HetznerCluster.Namespace, Name: clusterScope.HetznerCluster.Spec.HetznerSecret.Name, } secretManager := secretutil.NewSecretManager(clusterScope.Logger, clusterScope.Client, clusterScope.APIReader) - tokenSecret, err := secretManager.AcquireSecret(ctx, tokenSecretName, clusterScope.HetznerCluster, false, clusterScope.HetznerCluster.DeletionTimestamp.IsZero()) + mgtSecret, err := secretManager.AcquireSecret(ctx, mgtSecretName, clusterScope.HetznerCluster, false, clusterScope.HetznerCluster.DeletionTimestamp.IsZero()) if err != nil { return fmt.Errorf("failed to acquire secret: %w", err) } - hetznerToken, keyExists := tokenSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken] + hcloudToken, keyExists := mgtSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken] if !keyExists { return fmt.Errorf("error key %s does not exist in secret/%s: %w", clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken, - tokenSecretName, + mgtSecretName, err, ) } - if secret.Data == nil { - secret.Data = make(map[string][]byte) + if wlSecret.Data == nil { + wlSecret.Data = make(map[string][]byte) } - secret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken] = hetznerToken + wlSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken] = hcloudToken + + // upstream hcloud-ccm uses by default the secret key "token", while the old Syself ccm used + // "hcloud". For compatibility, we create always the other key, too. + for _, key := range []string{"token", "hcloud"} { + wlSecret.Data[key] = hcloudToken + } // Save robot credentials if available if clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotUser != "" { - robotUserName := tokenSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotUser] - secret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotUser] = robotUserName - robotPassword := tokenSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotPassword] - secret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotPassword] = robotPassword + robotUserName := mgtSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotUser] + wlSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotUser] = robotUserName + robotPassword := mgtSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotPassword] + wlSecret.Data[clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HetznerRobotPassword] = robotPassword } // Save network ID in secret if clusterScope.HetznerCluster.Spec.HCloudNetwork.Enabled { - secret.Data["network"] = []byte(strconv.FormatInt(clusterScope.HetznerCluster.Status.Network.ID, 10)) + wlSecret.Data["network"] = []byte(strconv.FormatInt(clusterScope.HetznerCluster.Status.Network.ID, 10)) } // Save api server information - secret.Data["apiserver-host"] = []byte(clusterScope.HetznerCluster.Spec.ControlPlaneEndpoint.Host) - secret.Data["apiserver-port"] = []byte(strconv.Itoa(int(clusterScope.HetznerCluster.Spec.ControlPlaneEndpoint.Port))) + wlSecret.Data["apiserver-host"] = []byte(clusterScope.HetznerCluster.Spec.ControlPlaneEndpoint.Host) + wlSecret.Data["apiserver-port"] = []byte(strconv.Itoa(int(clusterScope.HetznerCluster.Spec.ControlPlaneEndpoint.Port))) + + notes := []string{ + "This secret gets reconciled by Cluster API Provider Hetzner.", + } + + if clusterScope.HetznerCluster.Spec.HetznerSecret.Name != "hcloud" { + notes = append(notes, fmt.Sprintf("We recommend to use 'hcloud' for hetznercluster.spec.hetznerSecret.name (not %q).", + clusterScope.HetznerCluster.Spec.HetznerSecret.Name)) + } + + if clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken != "token" { + notes = append(notes, fmt.Sprintf("We recommend to use 'token' for hetznercluster.spec.hetznerSecret.key.hcloudToken (not %q).", clusterScope.HetznerCluster.Spec.HetznerSecret.Key.HCloudToken)) + } + + // Make things more obvious for people new to caph: + wlSecret.Data["note"] = []byte(strings.Join(notes, " ")) return nil }) if err != nil { - return reconcile.Result{}, fmt.Errorf("failed to create or update secret: %w", err) + return fmt.Errorf("failed to create or update secret: %w", err) } - return res, nil + return nil } func (r *HetznerClusterReconciler) reconcileTargetClusterManager(ctx context.Context, clusterScope *scope.ClusterScope) (res reconcile.Result, err error) { diff --git a/docs/caph/02-topics/05-baremetal/03-creating-workload-cluster.md b/docs/caph/02-topics/05-baremetal/03-creating-workload-cluster.md index e0811a7a8..0dda41a09 100644 --- a/docs/caph/02-topics/05-baremetal/03-creating-workload-cluster.md +++ b/docs/caph/02-topics/05-baremetal/03-creating-workload-cluster.md @@ -124,8 +124,8 @@ default my-cluster-control-plane-qwsq6 my-cluster my-cluster-control-pla default my-cluster-md-0-2xgj5-c5bhc my-cluster my-cluster-md-0-6xttr hcloud://45443694 Running 10h v1.31.6 default my-cluster-md-0-2xgj5-rbnbw my-cluster my-cluster-md-0-fdq9l hcloud://45443693 Running 10h v1.31.6 default my-cluster-md-0-2xgj5-tl2jr my-cluster my-cluster-md-0-59cgw hcloud://45443692 Running 10h v1.31.6 -default my-cluster-md-1-cp2fd-7nld7 my-cluster bm-my-cluster-md-1-d7526 hcloud://bm-2317525 Running 9h v1.31.6 -default my-cluster-md-1-cp2fd-n74sm my-cluster bm-my-cluster-md-1-l5dnr hcloud://bm-2105469 Running 10h v1.31.6 +default my-cluster-md-1-cp2fd-7nld7 my-cluster bm-my-cluster-md-1-d7526 hrobot://2317525 Running 9h v1.31.6 +default my-cluster-md-1-cp2fd-n74sm my-cluster bm-my-cluster-md-1-l5dnr hrobot://2105469 Running 10h v1.31.6 ``` -Please note that hcloud servers are prefixed with `hcloud://` and baremetal servers are prefixed with `hcloud://bm-`. +Please note that hcloud servers are prefixed with `hcloud://` and baremetal servers are prefixed with `hrobot://`. diff --git a/hack/get-kubeconfig-of-workload-cluster.sh b/hack/get-kubeconfig-of-workload-cluster.sh index 339deec52..d513b2aa2 100755 --- a/hack/get-kubeconfig-of-workload-cluster.sh +++ b/hack/get-kubeconfig-of-workload-cluster.sh @@ -16,12 +16,26 @@ set -euo pipefail -if [ -z "$CLUSTER_NAME" ]; then +if [ -z "${CLUSTER_NAME:-}" ]; then echo "env var CLUSTER_NAME is missing. Failed to get kubeconfig of workload cluster" exit 1 fi + +if [ -z "${KUBECONFIG:-}" ]; then + echo "env var KUBECONFIG (for mgt-cluster) is missing. Failed to get kubeconfig of workload cluster" + exit 1 +fi + +if [[ ! -e $KUBECONFIG ]]; then + echo "KUBECONFIG=$KUBECONFIG file does not exist! Failed to get kubeconfig of workload cluster" + exit 1 +fi + kubeconfig=".workload-cluster-kubeconfig.yaml" -new_content="$(kubectl get secrets "${CLUSTER_NAME}-kubeconfig" -ojsonpath='{.data.value}' | base64 -d)" +if ! new_content="$(kubectl get secrets "${CLUSTER_NAME}-kubeconfig" -ojsonpath='{.data.value}' 2>/dev/null | base64 -d)"; then + echo "error: Failed to get kubeconfig of wl-cluster" + exit 1 +fi if [ -z "$new_content" ]; then echo "failed to get kubeconfig of workload cluster" diff --git a/pkg/scope/cluster.go b/pkg/scope/cluster.go index 9c73dd40d..7f9790930 100644 --- a/pkg/scope/cluster.go +++ b/pkg/scope/cluster.go @@ -145,7 +145,7 @@ func (s *ClusterScope) ControlPlaneAPIEndpointPort() int32 { return int32(s.HetznerCluster.Spec.ControlPlaneLoadBalancer.Port) //nolint:gosec // Validation for the port range (1 to 65535) is already done via kubebuilder. } -// ClientConfig return a kubernetes client config for the cluster context. +// ClientConfig return a kubernetes client config for the workload cluster. func (s *ClusterScope) ClientConfig(ctx context.Context) (clientcmd.ClientConfig, error) { return workloadClientConfigFromKubeconfigSecret(ctx, s.Logger, s.Client, s.APIReader, s.Cluster, s.HetznerCluster) } diff --git a/pkg/services/baremetal/baremetal/baremetal.go b/pkg/services/baremetal/baremetal/baremetal.go index 50a8f9275..200a2551c 100644 --- a/pkg/services/baremetal/baremetal/baremetal.go +++ b/pkg/services/baremetal/baremetal/baremetal.go @@ -55,9 +55,6 @@ import ( // TODO: Implement logic for removal of unpaid servers. const ( - // providerIDPrefix is a prefix for ProviderID. - providerIDPrefix = "hcloud://" - // requeueAfter gives the duration of time until the next reconciliation should be performed. requeueAfter = time.Second * 30 @@ -685,7 +682,7 @@ func (s *Service) setProviderID(ctx context.Context) error { } // set providerID - providerID := providerIDFromServerID(host.Spec.ServerID) + providerID := fmt.Sprintf("hrobot://%d", host.Spec.ServerID) s.scope.BareMetalMachine.Spec.ProviderID = &providerID s.scope.BareMetalMachine.Status.Phase = clusterv1.MachinePhaseRunning @@ -900,10 +897,6 @@ func checkForRequeueError(err error, errMessage string) (res reconcile.Result, r return reconcile.Result{}, fmt.Errorf("%s: %w", errMessage, err) } -func providerIDFromServerID(serverID int) string { - return fmt.Sprintf("%s%s%d", providerIDPrefix, infrav1.BareMetalHostNamePrefix, serverID) -} - func analyzePatchError(err error, ignoreNotFound bool) error { if apierrors.IsConflict(err) { return &scope.RequeueAfterError{} diff --git a/pkg/services/baremetal/baremetal/baremetal_test.go b/pkg/services/baremetal/baremetal/baremetal_test.go index 6bbb0e20c..d121a24ba 100644 --- a/pkg/services/baremetal/baremetal/baremetal_test.go +++ b/pkg/services/baremetal/baremetal/baremetal_test.go @@ -702,10 +702,6 @@ var _ = Describe("Test ensureClusterLabel", func() { ) }) -var _ = Describe("Test providerIDFromServerID", func() { - Expect(providerIDFromServerID(42)).To(Equal("hcloud://bm-42")) -}) - var _ = Describe("Test hostKey", func() { host := &infrav1.HetznerBareMetalHost{} host.Namespace = "namespace" diff --git a/pkg/services/hcloud/util/utils.go b/pkg/services/hcloud/util/utils.go index 54dc74ada..b4585d318 100644 --- a/pkg/services/hcloud/util/utils.go +++ b/pkg/services/hcloud/util/utils.go @@ -45,15 +45,23 @@ func ProviderIDFromServerID(serverID int) string { return fmt.Sprintf("%s%v", providerIDPrefix, serverID) } -// ServerIDFromProviderID returns the serverID from a providerID. +// ServerIDFromProviderID returns the serverID from a providerID. This is used for hcloud machines +// only. The format must be "hcloud://NNN". func ServerIDFromProviderID(providerID *string) (int64, error) { if providerID == nil { return 0, ErrNilProviderID } + stringParts := strings.Split(*providerID, "://") if len(stringParts) != 2 || stringParts[0] == "" || stringParts[1] == "" { return 0, ErrInvalidProviderID } + + // Check that HCloud ProviderID starts with "hcloud" + if stringParts[0] != "hcloud" { + return 0, ErrInvalidProviderID + } + idString := stringParts[1] id, err := strconv.ParseInt(idString, 10, 64) if err != nil {