misc fixes

Signed-off-by: Bharath Nallapeta <[email protected]>

Author: bnallapeta

hack/install-kamaji.sh

@@ -16,6 +16,12 @@
 
 set -euo pipefail
 
+KUBECONFIG_ARG=""
+if [ -n "${1-}" ]; then
+    echo "Using kubeconfig: ${1}"
+    KUBECONFIG_ARG="--kubeconfig ${1}"
+fi
+
 # Simple Kamaji installation using Helm
 # This script installs Kamaji v1.0.0 in the management cluster
 
@@ -35,15 +41,16 @@ helm install kamaji clastix/kamaji \
   --version ${KAMAJI_VERSION} \
   --namespace ${KAMAJI_NAMESPACE} \
   --create-namespace \
+  ${KUBECONFIG_ARG} \
   --wait
 
 # Verify installation
 echo "Verifying Kamaji installation..."
-kubectl wait --for=condition=ready pod -l app.kubernetes.io/name=kamaji -n ${KAMAJI_NAMESPACE} --timeout=300s
+kubectl wait --for=condition=ready pod -l app.kubernetes.io/name=kamaji -n ${KAMAJI_NAMESPACE} --timeout=300s ${KUBECONFIG_ARG}
 
 # Create default datastore
 echo "Creating default datastore..."
-cat <<EOF | kubectl apply -f -
+cat <<EOF | kubectl apply -f - ${KUBECONFIG_ARG}
 apiVersion: kamaji.clastix.io/v1alpha1
 kind: DataStore
 metadata:

test/e2e/suites/hcp/hcp_suite_test.go

@@ -24,7 +24,6 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -33,9 +32,14 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/test/framework/clusterctl"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-openstack/test/e2e/shared"
 )
 
@@ -118,50 +122,18 @@ var _ = Describe("Hosted Control Plane tests", func() {
 			workloadCluster := e2eCtx.Environment.BootstrapClusterProxy.GetWorkloadCluster(ctx, namespace.Name, managementClusterName)
 			managementKubeconfig := workloadCluster.GetKubeconfigPath()
 
-			By("Installing Kamaji v1.0.0 on management cluster using Helm")
-			shared.Logf("Installing Kamaji v1.0.0 on management cluster")
-
-			// Install Kamaji using Helm
-			cmd := exec.Command("helm", "repo", "add", "clastix", "https://clastix.github.io/charts")
-			output, err := cmd.CombinedOutput()
-			shared.Logf("Helm repo add output: %s", string(output))
-			Expect(err).ToNot(HaveOccurred(), "Failed to add Clastix Helm repo: %v", err)
-
-			cmd = exec.Command("helm", "repo", "update")
-			output, err = cmd.CombinedOutput()
-			shared.Logf("Helm repo update output: %s", string(output))
-			Expect(err).ToNot(HaveOccurred(), "Failed to update Helm repos: %v", err)
-
-			cmd = exec.Command("helm", "install", "kamaji", "clastix/kamaji",
-				"--version", "v1.0.0",
-				"--namespace", "kamaji-system",
-				"--create-namespace",
-				"--kubeconfig", managementKubeconfig,
-				"--wait", "--timeout", "10m")
-			output, err = cmd.CombinedOutput()
-			shared.Logf("Kamaji installation output: %s", string(output))
-			Expect(err).ToNot(HaveOccurred(), "Failed to install Kamaji: %v", err)
-
-			By("Creating default DataStore for Kamaji")
-			datastoreYaml := `apiVersion: kamaji.clastix.io/v1alpha1
-kind: DataStore
-metadata:
-  name: default
-  namespace: kamaji-system
-spec:
-  driver: etcd
-  endpoints:
-  - kamaji-etcd.kamaji-system.svc.cluster.local:2379`
-
-			cmd = exec.Command("kubectl", "apply", "--kubeconfig", managementKubeconfig, "-f", "-")
-			cmd.Stdin = strings.NewReader(datastoreYaml)
-			output, err = cmd.CombinedOutput()
-			shared.Logf("DataStore creation output: %s", string(output))
-			Expect(err).ToNot(HaveOccurred(), "Failed to create DataStore: %v", err)
+			By("Installing Kamaji v1.0.0 on management cluster using shell script")
+			shared.Logf("Installing Kamaji via hack/install-kamaji.sh on management cluster: %s", managementClusterName)
+
+			installCmd := exec.Command("../../../../hack/install-kamaji.sh", managementKubeconfig)
+			output, err := installCmd.CombinedOutput()
+			shared.Logf("Kamaji installation script output:\n%s", string(output))
+			Expect(err).ToNot(HaveOccurred(), "Failed to install Kamaji using script")
 
 			By("Waiting for Kamaji to be ready")
-			// Give Kamaji some time to initialize
-			time.Sleep(30 * time.Second)
+			// The script waits for pods to be ready, but a small extra delay can prevent race conditions
+			// where the webhook is not yet fully available for the TenantControlPlane.
+			time.Sleep(10 * time.Second)
 
 			By("Creating workload cluster with Kamaji control plane")
 			shared.Logf("Creating HCP workload cluster: %s", workloadClusterName)
@@ -203,8 +175,53 @@ spec:
 				WorkloadClusterProxy: workloadClusterProxy,
 				Namespace:            namespace.Name,
 				ClusterName:          workloadClusterName,
+				E2EContext:           e2eCtx,
 			})
 
+			By("Testing terminal error for missing network")
+			// This test intentionally creates a machine that should fail to validate the terminal error handling
+			func() {
+				// Get the OpenStackCluster and patch its status to remove the network
+				openStackCluster := &infrav1.OpenStackCluster{}
+				err := workloadClusterProxy.GetClient().Get(ctx, types.NamespacedName{Name: workloadClusterName, Namespace: namespace.Name}, openStackCluster)
+				Expect(err).ToNot(HaveOccurred())
+
+				patch := client.MergeFrom(openStackCluster.DeepCopy())
+				openStackCluster.Status.Network = nil
+				Expect(workloadClusterProxy.GetClient().Status().Patch(ctx, openStackCluster, patch)).To(Succeed())
+
+				// Create a machine without a network defined in its spec. This should fail because the cluster network is also gone.
+				machineName := fmt.Sprintf("%s-terminal-test", workloadClusterName)
+				openStackMachine := &infrav1.OpenStackMachine{
+					ObjectMeta: metav1.ObjectMeta{Name: machineName, Namespace: namespace.Name},
+					Spec: infrav1.OpenStackMachineSpec{
+						Flavor:     e2eCtx.E2EConfig.Variables[shared.OpenstackNodeMachineFlavor],
+						Image:      infrav1.ImageParam{Filter: &infrav1.ImageFilter{Name: &e2eCtx.E2EConfig.Variables[shared.OpenstackImageName]}},
+						SSHKeyName: e2eCtx.E2EConfig.Variables[shared.OpenstackSSHKeyName],
+					},
+				}
+				Expect(workloadClusterProxy.GetClient().Create(ctx, openStackMachine)).To(Succeed())
+
+				// Assert that the machine gets the terminal error condition
+				g := NewWithT(GinkgoT())
+				g.Eventually(func() (bool, error) {
+					err := workloadClusterProxy.GetClient().Get(ctx, client.ObjectKeyFromObject(openStackMachine), openStackMachine)
+					if err != nil {
+						return false, err
+					}
+					for _, condition := range openStackMachine.Status.Conditions {
+						if condition.Type == clusterv1.ReadyCondition && condition.Status == corev1.ConditionFalse && condition.Severity == clusterv1.ConditionSeverityError && condition.Reason == infrav1.InvalidMachineSpecReason {
+							shared.Logf("Found terminal condition with correct reason: %s", condition.Message)
+							return true, nil
+						}
+					}
+					return false, nil
+				}, 10*time.Minute, 15*time.Second).Should(BeTrue(), "OpenStackMachine should have a terminal error condition for missing network")
+
+				// Clean up the test machine
+				Expect(workloadClusterProxy.GetClient().Delete(ctx, openStackMachine)).To(Succeed())
+			}()
+
 			By("Testing Konnectivity connectivity")
 			ValidateKonectivityConnectivity(ctx, NetworkValidationInput{
 				WorkloadClusterProxy: workloadClusterProxy,

test/e2e/suites/hcp/network_validation_test.go

@@ -21,24 +21,31 @@ package hcp
 
 import (
 	"context"
+	"fmt"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-openstack/test/e2e/shared"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/test/framework"
 )
 
 // NetworkValidationInput contains the input for network validation tests
 type NetworkValidationInput struct {
 	WorkloadClusterProxy *shared.ClusterProxy
 	Namespace            string
 	ClusterName          string
+	E2EContext           *shared.E2EContext
 }
 
 // ValidateNetworkConfiguration tests the specific network edge cases fixed in hcp-2380
@@ -54,6 +61,9 @@ func ValidateNetworkConfiguration(ctx context.Context, input NetworkValidationIn
 	By("Testing security group precedence")
 	validateSecurityGroupPrecedence(ctx, input)
 
+	By("Testing security group precedence with a live client")
+	validateSecurityGroupPrecedenceWithClient(ctx, input)
+
 	By("Testing port configuration edge cases")
 	validatePortConfigurationEdgeCases(ctx, input)
 
@@ -179,6 +189,116 @@ func validateSecurityGroupPrecedence(ctx context.Context, input NetworkValidatio
 	}
 }
 
+// validateSecurityGroupPrecedenceWithClient creates a machine with a specific security group
+// and verifies with the OpenStack client that it's the only one applied.
+func validateSecurityGroupPrecedenceWithClient(ctx context.Context, input NetworkValidationInput) {
+	shared.Logf("Validating security group precedence with a live client")
+
+	// Get a compute client
+	computeClient, err := shared.NewComputeClient(input.E2EContext)
+	Expect(err).ToNot(HaveOccurred(), "Failed to create compute client")
+
+	// Define a new security group to be used exclusively for this test
+	openStackCluster := &infrav1.OpenStackCluster{}
+	err = input.WorkloadClusterProxy.GetClient().Get(ctx, types.NamespacedName{
+		Namespace: input.Namespace,
+		Name:      input.ClusterName,
+	}, openStackCluster)
+	Expect(err).ToNot(HaveOccurred(), "Failed to get OpenStackCluster")
+
+	sgName := "e2e-sg-override-test"
+	sg, err := shared.CreateSecurityGroup(input.E2EContext, sgName)
+	Expect(err).ToNot(HaveOccurred(), "Failed to create security group for test")
+	defer func() {
+		Expect(shared.DeleteSecurityGroup(input.E2EContext, sg.ID)).To(Succeed())
+	}()
+
+	// Create a new Machine and OpenStackMachine with the override security group
+	machineName := fmt.Sprintf("%s-sg-override", input.ClusterName)
+	shared.Logf("Creating machine %s with security group %s", machineName, sg.Name)
+
+	machine := &clusterv1.Machine{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      machineName,
+			Namespace: input.Namespace,
+			Labels: map[string]string{
+				clusterv1.ClusterNameLabel: input.ClusterName,
+			},
+		},
+		Spec: clusterv1.MachineSpec{
+			ClusterName: input.ClusterName,
+			Version:     ptr.To(e2eCtx.E2EConfig.Variables[shared.KubernetesVersion]),
+			Bootstrap: clusterv1.Bootstrap{
+				ConfigRef: &corev1.ObjectReference{
+					APIVersion: "bootstrap.cluster.x-k8s.io/v1beta1",
+					Kind:       "KubeadmConfigTemplate",
+					Name:       fmt.Sprintf("%s-md-0", input.ClusterName),
+				},
+			},
+			InfrastructureRef: corev1.ObjectReference{
+				APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
+				Kind:       "OpenStackMachine",
+				Name:       machineName,
+			},
+		},
+	}
+
+	openStackMachine := &infrav1.OpenStackMachine{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      machineName,
+			Namespace: input.Namespace,
+		},
+		Spec: infrav1.OpenStackMachineSpec{
+			// Explicitly set the security group, overriding any cluster defaults
+			SecurityGroups: []infrav1.SecurityGroupParam{
+				{ID: &sg.ID},
+			},
+			Flavor:     e2eCtx.E2EConfig.Variables[shared.OpenstackNodeMachineFlavor],
+			Image:      infrav1.ImageParam{Filter: &infrav1.ImageFilter{Name: &e2eCtx.E2EConfig.Variables[shared.OpenstackImageName]}},
+			SSHKeyName: e2eCtx.E2EConfig.Variables[shared.OpenstackSSHKeyName],
+		},
+	}
+
+	// Create the resources
+	err = input.WorkloadClusterProxy.GetClient().Create(ctx, openStackMachine)
+	Expect(err).ToNot(HaveOccurred(), "Failed to create OpenStackMachine")
+	err = input.WorkloadClusterProxy.GetClient().Create(ctx, machine)
+	Expect(err).ToNot(HaveOccurred(), "Failed to create Machine")
+
+	// Wait for the machine to get an instance ID and become ready
+	shared.Logf("Waiting for machine %s to become ready", machineName)
+	framework.WaitForMachineReady(ctx, framework.WaitForMachineReadyInput{
+		Getter:    input.WorkloadClusterProxy.GetClient(),
+		Cluster:   &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: input.ClusterName, Namespace: input.Namespace}},
+		Machine:   machine,
+		Timeout:   20 * time.Minute,
+		Intervals: e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes"),
+	})
+
+	// Get the updated OpenStackMachine to find its instance ID
+	err = input.WorkloadClusterProxy.GetClient().Get(ctx, client.ObjectKeyFromObject(openStackMachine), openStackMachine)
+	Expect(err).ToNot(HaveOccurred(), "Failed to get updated OpenStackMachine")
+	Expect(openStackMachine.Status.InstanceID).ToNot(BeNil(), "InstanceID should not be nil")
+
+	// Use the compute client to verify the security groups on the live instance
+	shared.Logf("Verifying security groups on instance %s", *openStackMachine.Status.InstanceID)
+	server, err := servers.Get(computeClient, *openStackMachine.Status.InstanceID).Extract()
+	Expect(err).ToNot(HaveOccurred(), "Failed to get server details from OpenStack")
+
+	// Assert that the ONLY security group is the one we specified
+	Expect(server.SecurityGroups).To(HaveLen(1), "Should only have one security group")
+	Expect(server.SecurityGroups[0].(map[string]interface{})["name"]).To(Equal(sg.Name), "The applied security group should be the override one")
+
+	shared.Logf("Successfully verified security group override")
+
+	// Clean up the machine
+	shared.Logf("Deleting machine %s", machineName)
+	err = input.WorkloadClusterProxy.GetClient().Delete(ctx, machine)
+	Expect(err).ToNot(HaveOccurred(), "Failed to delete machine")
+	err = input.WorkloadClusterProxy.GetClient().Delete(ctx, openStackMachine)
+	Expect(err).ToNot(HaveOccurred(), "Failed to delete OpenStackMachine")
+}
+
 // validatePortConfigurationEdgeCases tests edge cases in port configuration
 func validatePortConfigurationEdgeCases(ctx context.Context, input NetworkValidationInput) {
 	shared.Logf("Validating port configuration edge cases")