Merge pull request #6 from Mirantis/bot/upstream-sync

Sync with upstream/main

Author: bnallapeta

.github/workflows/pr-dependabot.yaml

@@ -19,15 +19,15 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: Check out code into the Go module directory
-      uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
+      uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # tag=v5.0.0
     - name: Calculate go version
       id: vars
       run: echo "go_version=$(make go-version)" >> $GITHUB_OUTPUT
     - name: Set up Go
       uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # tag=v5.5.0
       with:
         go-version: ${{ steps.vars.outputs.go_version }}
-    - uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # tag=v4.2.3
+    - uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # tag=v4.2.4
       name: Restore go cache
       with:
         path: |

.github/workflows/release.yaml

@@ -17,7 +17,7 @@ jobs:
       - name: Set env
         run:  echo "RELEASE_TAG=${GITHUB_REF:10}" >> $GITHUB_ENV
       - name: checkout code
-        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
+        uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # tag=v5.0.0
         with:
           fetch-depth: 0
       - name: Calculate go version

.github/workflows/security-scan.yaml

@@ -18,7 +18,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - name: Check out code
-      uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
+      uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # tag=v5.0.0
       with:
         ref: ${{ matrix.branch }}
     - name: Calculate go version

.github/workflows/update-golangci-lint.yaml

@@ -19,7 +19,7 @@ jobs:
       current_version: ${{ steps.check_version.outputs.current_version }}
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
         with:
           fetch-depth: 0
       - name: Get latest golangci-lint version

Makefile

@@ -197,7 +197,7 @@ $(E2E_NO_ARTIFACT_TEMPLATES_DIR)/cluster-template.yaml: $(E2E_KUSTOMIZE_DIR)/wit
 $(E2E_NO_ARTIFACT_TEMPLATES_DIR)/cluster-template-%.yaml: $(E2E_KUSTOMIZE_DIR)/% $(KUSTOMIZE) FORCE
 	$(KUSTOMIZE) build "$<" > "$@"
 
-e2e-prerequisites: $(GINKGO) e2e-templates e2e-image test-e2e-image-prerequisites ## Build all artifacts required by e2e tests
+e2e-prerequisites: $(GINKGO) e2e-templates e2e-image ## Build all artifacts required by e2e tests
 
 # Can be run manually, e.g. via:
 # export OPENSTACK_CLOUD_YAML_FILE="$(pwd)/clouds.yaml"
@@ -221,12 +221,6 @@ build-e2e-tests: $(GINKGO)
 e2e-image: CONTROLLER_IMG_TAG = "gcr.io/k8s-staging-capi-openstack/capi-openstack-controller:e2e"
 e2e-image: docker-build
 
-# Pull all the images references in test/e2e/data/e2e_conf.yaml
-test-e2e-image-prerequisites:
-	docker pull registry.k8s.io/cluster-api/cluster-api-controller:v1.10.1
-	docker pull registry.k8s.io/cluster-api/kubeadm-bootstrap-controller:v1.10.1
-	docker pull registry.k8s.io/cluster-api/kubeadm-control-plane-controller:v1.10.1
-
 CONFORMANCE_E2E_ARGS ?= -kubetest.config-file=$(KUBETEST_CONF_PATH)
 CONFORMANCE_E2E_ARGS += $(E2E_ARGS)
 .PHONY: test-conformance

controllers/openstackcluster_controller.go

@@ -499,7 +499,10 @@ func (r *OpenStackClusterReconciler) reconcileBastionServer(ctx context.Context,
 	}
 
 	// If the bastion is found but the spec has changed, we need to delete it and reconcile.
-	bastionServerSpec := bastionToOpenStackServerSpec(openStackCluster)
+	bastionServerSpec, err := bastionToOpenStackServerSpec(openStackCluster)
+	if err != nil {
+		return nil, true, err
+	}
 	if !bastionNotFound && server != nil && !apiequality.Semantic.DeepEqual(bastionServerSpec, &server.Spec) {
 		scope.Logger().Info("Bastion spec has changed, re-creating the OpenStackServer object")
 		if err := r.deleteBastion(ctx, scope, cluster, openStackCluster); err != nil {
@@ -545,7 +548,10 @@ func (r *OpenStackClusterReconciler) getBastionServer(ctx context.Context, openS
 // createBastionServer creates the OpenStackServer object for the bastion server.
 // It returns the OpenStackServer object and an error if any.
 func (r *OpenStackClusterReconciler) createBastionServer(ctx context.Context, openStackCluster *infrav1.OpenStackCluster, cluster *clusterv1.Cluster) (*infrav1alpha1.OpenStackServer, error) {
-	bastionServerSpec := bastionToOpenStackServerSpec(openStackCluster)
+	bastionServerSpec, err := bastionToOpenStackServerSpec(openStackCluster)
+	if err != nil {
+		return nil, err
+	}
 	bastionServer := &infrav1alpha1.OpenStackServer{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
@@ -573,7 +579,7 @@ func (r *OpenStackClusterReconciler) createBastionServer(ctx context.Context, op
 
 // bastionToOpenStackServerSpec converts the OpenStackMachineSpec for the bastion to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec and an error if any.
-func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *infrav1alpha1.OpenStackServerSpec {
+func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) (*infrav1alpha1.OpenStackServerSpec, error) {
 	bastion := openStackCluster.Spec.Bastion
 	if bastion == nil {
 		bastion = &infrav1.Bastion{}
@@ -588,9 +594,12 @@ func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *i
 	if bastion.AvailabilityZone != nil {
 		az = *bastion.AvailabilityZone
 	}
-	openStackServerSpec := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network.ID)
+	openStackServerSpec, err := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network)
+	if err != nil {
+		return nil, err
+	}
 
-	return openStackServerSpec
+	return openStackServerSpec, nil
 }
 
 func bastionName(clusterResourceName string) string {

controllers/openstackmachine_controller.go

@@ -480,7 +480,18 @@ func (r *OpenStackMachineReconciler) getMachineServer(ctx context.Context, openS
 
 // openStackMachineSpecToOpenStackServerSpec converts an OpenStackMachineSpec to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec object and an error if there is any.
-func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, defaultNetworkID string) *infrav1alpha1.OpenStackServerSpec {
+func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, clusterNetwork *infrav1.NetworkStatusWithSubnets) (*infrav1alpha1.OpenStackServerSpec, error) {
+	// Determine default network ID if the cluster status exposes one.
+	var defaultNetworkID string
+	if clusterNetwork != nil {
+		defaultNetworkID = clusterNetwork.ID
+	}
+
+	// If no cluster network is available AND the machine spec did not define any ports with a network, we cannot choose a network.
+	if defaultNetworkID == "" && len(openStackMachineSpec.Ports) == 0 {
+		return nil, capoerrors.Terminal(infrav1.InvalidMachineSpecReason, "no network configured: cluster network is missing and machine spec does not define ports with a network")
+	}
+
 	openStackServerSpec := &infrav1alpha1.OpenStackServerSpec{
 		AdditionalBlockDevices:            openStackMachineSpec.AdditionalBlockDevices,
 		ConfigDrive:                       openStackMachineSpec.ConfigDrive,
@@ -522,7 +533,8 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
 		serverPorts = make([]infrav1.PortOpts, 1)
 	}
 	for i := range serverPorts {
-		if serverPorts[i].Network == nil {
+		// Only inject the default network when we actually have an ID.
+		if serverPorts[i].Network == nil && defaultNetworkID != "" {
 			serverPorts[i].Network = &infrav1.NetworkParam{
 				ID: &defaultNetworkID,
 			}
@@ -540,7 +552,7 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
 	}
 	openStackServerSpec.Ports = serverPorts
 
-	return openStackServerSpec
+	return openStackServerSpec, nil
 }
 
 // reconcileMachineServer reconciles the OpenStackServer object for the OpenStackMachine.
@@ -589,7 +601,10 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
 			}
 			return openStackCluster.Spec.IdentityRef
 		}()
-		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
+		machineServerSpec, err := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network)
+		if err != nil {
+			return nil, err
+		}
 		machineServer = &infrav1alpha1.OpenStackServer{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{

controllers/openstackmachine_controller_test.go

@@ -95,9 +95,11 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 	tags := []string{"tag1", "tag2"}
 	userData := &corev1.LocalObjectReference{Name: "server-data-secret"}
 	tests := []struct {
-		name string
-		spec *infrav1.OpenStackMachineSpec
-		want *infrav1alpha1.OpenStackServerSpec
+		name           string
+		spec           *infrav1.OpenStackMachineSpec
+		clusterNetwork *infrav1.NetworkStatusWithSubnets
+		want           *infrav1alpha1.OpenStackServerSpec
+		wantErr        bool
 	}{
 		{
 			name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
@@ -106,6 +108,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				Image:      image,
 				SSHKeyName: sshKeyName,
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				Flavor:      ptr.To(flavorName),
 				IdentityRef: identityRef,
@@ -128,6 +131,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 					},
 				},
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				Flavor:      ptr.To(flavorName),
 				IdentityRef: identityRef,
@@ -146,6 +150,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				Image:      image,
 				SSHKeyName: sshKeyName,
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				Flavor:      ptr.To(flavorName),
 				FlavorID:    ptr.To(flavorUUID),
@@ -164,6 +169,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				Image:      image,
 				SSHKeyName: sshKeyName,
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				FlavorID:    ptr.To(flavorUUID),
 				IdentityRef: identityRef,
@@ -174,12 +180,90 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				UserDataRef: userData,
 			},
 		},
+		{
+			name: "Cluster network nil, machine defines port network and overrides SG",
+			spec: &infrav1.OpenStackMachineSpec{
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+				}},
+				SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(extraSecurityGroupUUID)}},
+			},
+			clusterNetwork: nil,
+			want: &infrav1alpha1.OpenStackServerSpec{
+				IdentityRef: identityRef,
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+					SecurityGroups: []infrav1.SecurityGroupParam{
+						{ID: ptr.To(workerSecurityGroupUUID)},
+						{ID: ptr.To(extraSecurityGroupUUID)},
+					},
+				}},
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
+		{
+			name: "Cluster network nil, machine defines port network and falls back to cluster SG",
+			spec: &infrav1.OpenStackMachineSpec{
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+				}},
+			},
+			clusterNetwork: nil,
+			want: &infrav1alpha1.OpenStackServerSpec{
+				IdentityRef: identityRef,
+				Ports: []infrav1.PortOpts{{
+					Network:        &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+					SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(workerSecurityGroupUUID)}},
+				}},
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
+		{
+			name: "Error case: no cluster network and no machine ports",
+			spec: &infrav1.OpenStackMachineSpec{
+				Flavor:     ptr.To(flavorName),
+				Image:      image,
+				SSHKeyName: sshKeyName,
+				// No ports defined
+			},
+			clusterNetwork: nil,
+			want:           nil,
+			wantErr:        true,
+		},
+		{
+			name: "Empty cluster network ID, machine defines explicit ports",
+			spec: &infrav1.OpenStackMachineSpec{
+				Flavor: ptr.To(flavorName),
+				Image:  image,
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+				}},
+			},
+			clusterNetwork: &infrav1.NetworkStatusWithSubnets{NetworkStatus: infrav1.NetworkStatus{ID: ""}},
+			want: &infrav1alpha1.OpenStackServerSpec{
+				Flavor:      ptr.To(flavorName),
+				IdentityRef: identityRef,
+				Image:       image,
+				Ports: []infrav1.PortOpts{{
+					Network:        &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+					SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(workerSecurityGroupUUID)}},
+				}},
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
 	}
 	for i := range tests {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
-			spec := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, openStackCluster.Status.Network.ID)
-			if !reflect.DeepEqual(spec, tt.want) {
+			spec, err := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, tt.clusterNetwork)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("openStackMachineSpecToOpenStackServerSpec() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !tt.wantErr && !reflect.DeepEqual(spec, tt.want) {
 				t.Errorf("openStackMachineSpecToOpenStackServerSpec() got = %+v, want %+v", spec, tt.want)
 			}
 		})

docs/book/src/SUMMARY.md

@@ -5,6 +5,7 @@
 - [Configuration](clusteropenstack/configuration.md)
 - [Topics](./topics/index.md)
     - [external cloud provider](./topics/external-cloud-provider.md)
+    - [hosted control plane](./topics/hosted-control-plane.md)
     - [move from bootstrap](./topics/mover.md)
     - [trouble shooting](./topics/troubleshooting.md)
     - [CRD Changes](./topics/crd-changes/index.md)