bastion: document and guard how bastion is removed or updated

We now have a webhook that checks that a bastion has been disabled if a
change has to be made (update or delete) in the bastion field.

We also document it better.

Author: EmilienM

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"`
 

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{}

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) {

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

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

controllers/openstackcluster_controller.go

@@ -263,6 +263,11 @@ func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackClust
 			}
 		}
 
+		// To remove the bastion, we need to get the instance spec. So when a user
+		// wants to remove the OpenStackCluster.Spec.Bastion, they first need to set
+		// the OpenStackCluster.Spec.Bastion.Enabled to false so here we can still get
+		// the instance spec and delete the bastion. Then changes can be made to the
+		// OpenStackCluster.Spec.Bastion.
 		instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
 		if err = computeService.DeleteInstance(openStackCluster, openStackCluster, instanceStatus, instanceSpec); err != nil {
 			handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete bastion: %w", err))

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)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
@@ -654,6 +656,16 @@ 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. Then, you can make changes to the instance spec and re-enable the bastion host by setting `enabled: true` and by modifying the `OpenStackCluster.Spec.Bastion.Instance` field.
+
+### Disabling the bastion
+
+To disable the bastion host, set `enabled: false` in the `OpenStackCluster.Spec.Bastion` field. The bastion host will be deleted.
+Then you can 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: