Merge pull request #8450 from patrickdillon/debug-gather

OCPBUGS-34295: wait for ip addresses to be present on machines

Author: patrickdillon

pkg/infrastructure/aws/clusterapi/aws.go

@@ -40,6 +40,10 @@ type Provider struct{}
 // Name gives the name of the provider, AWS.
 func (*Provider) Name() string { return awstypes.Name }
 
+// BootstrapHasPublicIP indicates that machine ready checks
+// should wait for an ExternalIP in the status.
+func (*Provider) BootstrapHasPublicIP() bool { return true }
+
 // PreProvision creates the IAM roles used by all nodes in the cluster.
 func (*Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {
 	if err := createIAMRoles(ctx, in.InfraID, in.InstallConfig); err != nil {

pkg/infrastructure/azure/azure.go

@@ -51,6 +51,10 @@ func (p *Provider) Name() string {
 	return aztypes.Name
 }
 
+// BootstrapHasPublicIP indicates that an ExternalIP is not
+// required in the machine ready checks.
+func (*Provider) BootstrapHasPublicIP() bool { return false }
+
 // PreProvision is called before provisioning using CAPI controllers has begun.
 func (p *Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {
 	session, err := in.InstallConfig.Azure.Session()

pkg/infrastructure/clusterapi/clusterapi.go

@@ -15,6 +15,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	utilkubeconfig "sigs.k8s.io/cluster-api/util/kubeconfig"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -269,36 +270,39 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset
 	masterIgnSecret := IgnitionSecret(masterIgnAsset.Files()[0].Data, clusterID.InfraID, "master")
 	machineManifests = append(machineManifests, bootstrapIgnSecret, masterIgnSecret)
 
-	timer.StartTimer(machineStage)
 	// Create the machine manifests.
+	timer.StartTimer(machineStage)
+	machineNames := []string{}
+
 	for _, m := range machineManifests {
 		m.SetNamespace(capiutils.Namespace)
 		if err := cl.Create(ctx, m); err != nil {
 			return fileList, fmt.Errorf("failed to create control-plane manifest: %w", err)
 		}
 		i.appliedManifests = append(i.appliedManifests, m)
+
+		if machine, ok := m.(*clusterv1.Machine); ok {
+			machineNames = append(machineNames, machine.Name)
+		}
 		logrus.Infof("Created manifest %+T, namespace=%s name=%s", m, m.GetNamespace(), m.GetName())
 	}
 
 	{
-		masterCount := int64(1)
-		if reps := installConfig.Config.ControlPlane.Replicas; reps != nil {
-			masterCount = *reps
-		}
-
 		untilTime := time.Now().Add(timeout)
 		timezone, _ := untilTime.Zone()
-		logrus.Infof("Waiting up to %v (until %v %s) for machines to provision...", timeout, untilTime.Format(time.Kitchen), timezone)
+		reqBootstrapPubIP := installConfig.Config.Publish == types.ExternalPublishingStrategy && i.impl.BootstrapHasPublicIP()
+		logrus.Infof("Waiting up to %v (until %v %s) for machines %v to provision...", timeout, untilTime.Format(time.Kitchen), timezone, machineNames)
 		if err := wait.ExponentialBackoffWithContext(ctx, wait.Backoff{
 			Duration: time.Second * 10,
 			Factor:   float64(1.5),
 			Steps:    32,
 			Cap:      timeout,
 		}, func(ctx context.Context) (bool, error) {
-			for i := int64(0); i < masterCount; i++ {
+			allReady := true
+			for _, machineName := range machineNames {
 				machine := &clusterv1.Machine{}
 				if err := cl.Get(ctx, client.ObjectKey{
-					Name:      fmt.Sprintf("%s-%s-%d", clusterID.InfraID, "master", i),
+					Name:      machineName,
 					Namespace: capiutils.Namespace,
 				}, machine); err != nil {
 					if apierrors.IsNotFound(err) {
@@ -307,15 +311,18 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset
 					}
 					return false, err
 				}
-				if machine.Status.Phase != string(clusterv1.MachinePhaseProvisioned) &&
-					machine.Status.Phase != string(clusterv1.MachinePhaseRunning) {
-					return false, nil
-				} else if machine.Status.Phase == string(clusterv1.MachinePhaseFailed) {
-					return false, fmt.Errorf("machine %s failed to provision: %q", machine.Name, *machine.Status.FailureMessage)
+				reqPubIP := reqBootstrapPubIP && machineName == capiutils.GenerateBoostrapMachineName(clusterID.InfraID)
+				ready, err := checkMachineReady(machine, reqPubIP)
+				if err != nil {
+					return false, fmt.Errorf("failed waiting for machines: %w", err)
+				}
+				if !ready {
+					allReady = false
+				} else {
+					logrus.Debugf("Machine %s is ready. Phase: %s", machine.Name, machine.Status.Phase)
 				}
-				logrus.Debugf("Machine %s is ready. Phase: %s", machine.Name, machine.Status.Phase)
 			}
-			return true, nil
+			return allReady, nil
 		}); err != nil {
 			if wait.Interrupted(err) {
 				return fileList, fmt.Errorf("control-plane machines were not provisioned within %v: %w", timeout, err)
@@ -550,3 +557,37 @@ func (i *InfraProvider) collectManifests(ctx context.Context, cl client.Client)
 	}
 	return fileList, errorList
 }
+
+func checkMachineReady(machine *clusterv1.Machine, requirePublicIP bool) (bool, error) {
+	logrus.Debugf("Checking that machine %s has provisioned...", machine.Name)
+	if machine.Status.Phase != string(clusterv1.MachinePhaseProvisioned) &&
+		machine.Status.Phase != string(clusterv1.MachinePhaseRunning) {
+		logrus.Debugf("Machine %s has not yet provisioned: %s", machine.Name, machine.Status.Phase)
+		return false, nil
+	} else if machine.Status.Phase == string(clusterv1.MachinePhaseFailed) {
+		msg := ptr.Deref(machine.Status.FailureMessage, "machine.Status.FailureMessage was not set")
+		return false, fmt.Errorf("machine %s failed to provision: %s", machine.Name, msg)
+	}
+	logrus.Debugf("Machine %s has status: %s", machine.Name, machine.Status.Phase)
+	return hasRequiredIP(machine, requirePublicIP), nil
+}
+
+func hasRequiredIP(machine *clusterv1.Machine, requirePublicIP bool) bool {
+	logrus.Debugf("Checking that IP addresses are populated in the status of machine %s...", machine.Name)
+
+	for _, addr := range machine.Status.Addresses {
+		switch {
+		case len(addr.Address) == 0:
+			continue
+		case addr.Type == clusterv1.MachineExternalIP:
+			logrus.Debugf("Found external IP address: %s", addr.Address)
+			return true
+		case addr.Type == clusterv1.MachineInternalIP && !requirePublicIP:
+			logrus.Debugf("Found internal IP address: %s", addr.Address)
+			return true
+		}
+		logrus.Debugf("Checked IP %s: %s", addr.Type, addr.Address)
+	}
+	logrus.Debugf("Still waiting for machine %s to get required IPs", machine.Name)
+	return false
+}

pkg/infrastructure/clusterapi/types.go

@@ -18,6 +18,12 @@ import (
 type Provider interface {
 	// Name provides the name for the cloud platform.
 	Name() string
+
+	// BootstrapHasPublicIP indicates whether a public IP address
+	// is expected on the bootstrap node in a public cluster.
+	// When BootstrapHasPublicIP returns true, the machine ready checks
+	// wait for an ExternalIP address to be populated in the machine status.
+	BootstrapHasPublicIP() bool
 }
 
 // PreProvider defines the PreProvision hook, which is called prior to

pkg/infrastructure/gcp/clusterapi/clusterapi.go

@@ -37,6 +37,10 @@ func (p Provider) Name() string {
 	return gcptypes.Name
 }
 
+// BootstrapHasPublicIP indicates that machine ready checks
+// should wait for an ExternalIP in the status.
+func (Provider) BootstrapHasPublicIP() bool { return true }
+
 // PreProvision is called before provisioning using CAPI controllers has initiated.
 // GCP resources that are not created by CAPG (and are required for other stages of the install) are
 // created here using the gcp sdk.

pkg/infrastructure/ibmcloud/clusterapi/clusterapi.go

@@ -18,6 +18,10 @@ func (p Provider) Name() string {
 	return ibmcloudtypes.Name
 }
 
+// BootstrapHasPublicIP indicates that an ExternalIP is not
+// required in the machine ready checks.
+func (Provider) BootstrapHasPublicIP() bool { return false }
+
 // PreProvision creates the IBM Cloud objects required prior to running capibmcloud.
 func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {
 	return nil

pkg/infrastructure/nutanix/clusterapi/clusterapi.go

@@ -26,6 +26,10 @@ func (p Provider) Name() string {
 	return nutanixtypes.Name
 }
 
+// BootstrapHasPublicIP indicates that an ExternalIP is not
+// required in the machine ready checks.
+func (Provider) BootstrapHasPublicIP() bool { return false }
+
 // PreProvision creates the resources required prior to running capi nutanix controller.
 func (p Provider) PreProvision(ctx context.Context, in infracapi.PreProvisionInput) error {
 	// create categories with name "kubernetes-io-cluster-<cluster_id>" and values ["owned", "shared"].

pkg/infrastructure/openstack/clusterapi/clusterapi.go

@@ -31,6 +31,10 @@ func (p Provider) Name() string {
 	return openstack.Name
 }
 
+// BootstrapHasPublicIP indicates that an ExternalIP is not
+// required in the machine ready checks.
+func (Provider) BootstrapHasPublicIP() bool { return false }
+
 var _ clusterapi.PreProvider = Provider{}
 
 // PreProvision tags the VIP ports, and creates the security groups and the

pkg/infrastructure/powervs/clusterapi/powervs.go

@@ -38,6 +38,10 @@ func (p Provider) Name() string {
 	return powervstypes.Name
 }
 
+// BootstrapHasPublicIP indicates that an ExternalIP is not
+// required in the machine ready checks.
+func (Provider) BootstrapHasPublicIP() bool { return false }
+
 func leftInContext(ctx context.Context) time.Duration {
 	deadline, ok := ctx.Deadline()
 	if !ok {

pkg/infrastructure/vsphere/clusterapi/clusterapi.go

@@ -27,6 +27,10 @@ func (p Provider) Name() string {
 	return vsphere.Name
 }
 
+// BootstrapHasPublicIP indicates that an ExternalIP is not
+// required in the machine ready checks.
+func (Provider) BootstrapHasPublicIP() bool { return false }
+
 func initializeFoldersAndTemplates(ctx context.Context, cachedImage string, failureDomain vsphere.FailureDomain, session *session.Session, diskType vsphere.DiskType, clusterID, tagID string) error {
 	finder := session.Finder