diff --git a/api/v1alpha8/openstackcluster_types.go b/api/v1alpha8/openstackcluster_types.go index 67a1bc4a0a..a669c8317d 100644 --- a/api/v1alpha8/openstackcluster_types.go +++ b/api/v1alpha8/openstackcluster_types.go @@ -155,8 +155,8 @@ type OpenStackClusterSpec struct { // Bastion is the OpenStack instance to login the nodes // // As a rolling update is not ideal during a bastion host session, we - // prevent changes to a running bastion configuration. Set `enabled: false` to - // make changes. + // prevent changes to a running bastion configuration. To make changes, it's required + // to first set `enabled: false` which will remove the bastion and then changes can be made. //+optional Bastion *Bastion `json:"bastion,omitempty"` diff --git a/api/v1alpha8/openstackcluster_webhook.go b/api/v1alpha8/openstackcluster_webhook.go index 2344870c7d..b47c2ac612 100644 --- a/api/v1alpha8/openstackcluster_webhook.go +++ b/api/v1alpha8/openstackcluster_webhook.go @@ -116,6 +116,13 @@ func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warn r.Spec.APIServerPort = 0 } + // Allow to remove the bastion spec only if it was disabled before. + if r.Spec.Bastion == nil { + if old.Spec.Bastion != nil && old.Spec.Bastion.Enabled { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "bastion"), "cannot be removed before disabling it")) + } + } + // Allow changes to the bastion spec. old.Spec.Bastion = &Bastion{} r.Spec.Bastion = &Bastion{} diff --git a/api/v1alpha8/openstackcluster_webhook_test.go b/api/v1alpha8/openstackcluster_webhook_test.go index 27e8ab34db..734a388763 100644 --- a/api/v1alpha8/openstackcluster_webhook_test.go +++ b/api/v1alpha8/openstackcluster_webhook_test.go @@ -358,6 +358,42 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, wantErr: true, }, + { + name: "Removing OpenStackCluster.Spec.Bastion when it is enabled is not allowed", + oldTemplate: &OpenStackCluster{ + Spec: OpenStackClusterSpec{ + Bastion: &Bastion{ + Enabled: true, + Instance: OpenStackMachineSpec{ + Flavor: "m1.small", + Image: ImageFilter{Name: "ubuntu"}, + }, + }, + }, + }, + newTemplate: &OpenStackCluster{ + Spec: OpenStackClusterSpec{}, + }, + wantErr: true, + }, + { + name: "Removing OpenStackCluster.Spec.Bastion when it is disabled is allowed", + oldTemplate: &OpenStackCluster{ + Spec: OpenStackClusterSpec{ + Bastion: &Bastion{ + Enabled: false, + Instance: OpenStackMachineSpec{ + Flavor: "m1.small", + Image: ImageFilter{Name: "ubuntu"}, + }, + }, + }, + }, + newTemplate: &OpenStackCluster{ + Spec: OpenStackClusterSpec{}, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index 3caa2cbe2f..0e929c11c8 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -4899,8 +4899,8 @@ spec: As a rolling update is not ideal during a bastion host session, we - prevent changes to a running bastion configuration. Set `enabled: false` to - make changes. + prevent changes to a running bastion configuration. To make changes, it's required + to first set `enabled: false` which will remove the bastion and then changes can be made. properties: availabilityZone: type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 2cab684d19..ae5555b4a2 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2324,8 +2324,8 @@ spec: As a rolling update is not ideal during a bastion host session, we - prevent changes to a running bastion configuration. Set `enabled: false` to - make changes. + prevent changes to a running bastion configuration. To make changes, it's required + to first set `enabled: false` which will remove the bastion and then changes can be made. properties: availabilityZone: type: string diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 51361a1a85..d63a6692da 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -263,7 +263,10 @@ func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackClust } } - instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name) + instanceSpec, err := bastionToInstanceSpec(openStackCluster, cluster.Name) + if err != nil { + return err + } if err = computeService.DeleteInstance(openStackCluster, openStackCluster, instanceStatus, instanceSpec); err != nil { handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete bastion: %w", err)) return fmt.Errorf("failed to delete bastion: %w", err) @@ -346,7 +349,10 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl return reconcile.Result{}, err } - instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name) + instanceSpec, err := bastionToInstanceSpec(openStackCluster, cluster.Name) + if err != nil { + return reconcile.Result{}, err + } bastionHash, err := compute.HashInstanceSpec(instanceSpec) if err != nil { return reconcile.Result{}, fmt.Errorf("failed computing bastion hash from instance spec: %w", err) @@ -437,7 +443,15 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl return ctrl.Result{}, nil } -func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterName string) *compute.InstanceSpec { +func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterName string) (*compute.InstanceSpec, error) { + if openStackCluster.Spec.Bastion == nil { + return nil, fmt.Errorf("bastion spec is nil") + } + + if openStackCluster.Status.Bastion == nil { + return nil, fmt.Errorf("bastion status is nil") + } + instanceSpec := &compute.InstanceSpec{ Name: bastionName(clusterName), Flavor: openStackCluster.Spec.Bastion.Instance.Flavor, @@ -458,7 +472,7 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterNa instanceSpec.Ports = openStackCluster.Spec.Bastion.Instance.Ports - return instanceSpec + return instanceSpec, nil } func bastionName(clusterName string) string { diff --git a/docs/book/src/clusteropenstack/configuration.md b/docs/book/src/clusteropenstack/configuration.md index 1a075bc57e..3ae013333f 100644 --- a/docs/book/src/clusteropenstack/configuration.md +++ b/docs/book/src/clusteropenstack/configuration.md @@ -35,6 +35,8 @@ - [Custom pod network CIDR](#custom-pod-network-cidr) - [Accessing nodes through the bastion host via SSH](#accessing-nodes-through-the-bastion-host-via-ssh) - [Enabling the bastion host](#enabling-the-bastion-host) + - [Making changes to the bastion host](#making-changes-to-the-bastion-host) + - [Disabling the bastion](#disabling-the-bastion) - [Obtain floating IP address of the bastion node](#obtain-floating-ip-address-of-the-bastion-node) @@ -654,6 +656,19 @@ spec: If `managedSecurityGroups: true`, security group rule opening 22/tcp is added to security groups for bastion, controller, and worker nodes respectively. Otherwise, you have to add `securityGroups` to the `bastion` in `OpenStackCluster` spec and `OpenStackMachineTemplate` spec template respectively. +### Making changes to the bastion host + +Changes can be made to the bastion instance, like for example changing the flavor. +First, you have to disable the bastion host by setting `enabled: false` in the `OpenStackCluster.Spec.Bastion` field. +The bastion will be deleted, you can check the status of the bastion host by running `kubectl get openstackcluster` and looking at the `Bastion` field in status. +Once it's gone, you can re-enable the bastion host by setting `enabled: true` and then making changes to the bastion instance spec by modifying the `OpenStackCluster.Spec.Bastion.Instance` field. +The bastion host will be re-created with the new instance spec. + +### Disabling the bastion + +To disable the bastion host, set `enabled: false` in the `OpenStackCluster.Spec.Bastion` field. The bastion host will be deleted, you can check the status of the bastion host by running `kubectl get openstackcluster` and looking at the `Bastion` field in status. +Once it's gone, you can now remove the `OpenStackCluster.Spec.Bastion` field from the `OpenStackCluster` spec. + ### Obtain floating IP address of the bastion node Once the workload cluster is up and running after being configured for an SSH bastion host, you can use the kubectl get openstackcluster command to look up the floating IP address of the bastion host (make sure the kubectl context is set to the management cluster). The output will look something like this: