Merge pull request openshift#8812 from patrickdillon/capi-gather-interface

OCPBUGS-37540: Gather Azure Logs through load balancer

Author: openshift-merge-bot[bot]

pkg/infrastructure/aws/clusterapi/aws.go

@@ -46,9 +46,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should wait for an ExternalIP
+// in the status and use that when gathering bootstrap log bundles.
+func (*Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.ExternalIP }
 
 // PreProvision creates the IAM roles used by all nodes in the cluster.
 func (*Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {

pkg/infrastructure/azure/azure.go

@@ -67,9 +67,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should NOT wait for an ExternalIP
+// in the status and should use the API load balancer when gathering bootstrap log bundles.
+func (*Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.APILoadBalancer }
 
 // PreProvision is called before provisioning using CAPI controllers has begun.
 func (p *Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {

pkg/infrastructure/clusterapi/clusterapi.go

@@ -325,7 +325,7 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset
 	{
 		untilTime := time.Now().Add(provisionTimeout)
 		timezone, _ := untilTime.Zone()
-		reqBootstrapPubIP := installConfig.Config.Publish == types.ExternalPublishingStrategy && i.impl.BootstrapHasPublicIP()
+		reqBootstrapPubIP := installConfig.Config.Publish == types.ExternalPublishingStrategy && i.impl.PublicGatherEndpoint() == ExternalIP
 		logrus.Infof("Waiting up to %v (until %v %s) for machines %v to provision...", provisionTimeout, untilTime.Format(time.Kitchen), timezone, machineNames)
 		if err := wait.PollUntilContextTimeout(ctx, 15*time.Second, provisionTimeout, true,
 			func(ctx context.Context) (bool, error) {
@@ -504,22 +504,12 @@ func (i *InfraProvider) ExtractHostAddresses(dir string, config *types.InstallCo
 	manifestsDir := filepath.Join(dir, clusterapi.ArtifactsDir)
 	logrus.Debugf("Looking for machine manifests in %s", manifestsDir)
 
-	bootstrapFiles, err := filepath.Glob(filepath.Join(manifestsDir, "Machine\\-openshift\\-cluster\\-api\\-guests\\-*\\-bootstrap.yaml"))
-	if err != nil {
-		return fmt.Errorf("failed to list bootstrap manifests: %w", err)
-	}
-	logrus.Debugf("bootstrap manifests found: %v", bootstrapFiles)
-
-	if len(bootstrapFiles) != 1 {
-		return fmt.Errorf("wrong number of bootstrap manifests found: %v. Expected exactly one", bootstrapFiles)
-	}
-	addrs, err := extractIPAddress(bootstrapFiles[0])
+	addr, err := i.getBootstrapAddress(config, manifestsDir)
 	if err != nil {
-		return fmt.Errorf("failed to extract IP address for bootstrap: %w", err)
+		return fmt.Errorf("failed to get bootstrap address: %w", err)
 	}
-	logrus.Debugf("found bootstrap address: %s", addrs)
+	ha.Bootstrap = addr
 
-	ha.Bootstrap = prioritizeIPv4(config, addrs)
 	masterFiles, err := filepath.Glob(filepath.Join(manifestsDir, "Machine\\-openshift\\-cluster\\-api\\-guests\\-*\\-master\\-?.yaml"))
 	if err != nil {
 		return fmt.Errorf("failed to list master machine manifests: %w", err)
@@ -544,6 +534,30 @@ func (i *InfraProvider) ExtractHostAddresses(dir string, config *types.InstallCo
 	return nil
 }
 
+func (i *InfraProvider) getBootstrapAddress(config *types.InstallConfig, manifestsDir string) (string, error) {
+	// If the bootstrap node cannot have a public IP address, we
+	// SSH through the load balancer, as is this case on Azure.
+	if i.impl.PublicGatherEndpoint() == APILoadBalancer && config.Publish != types.InternalPublishingStrategy {
+		return fmt.Sprintf("api.%s", config.ClusterDomain()), nil
+	}
+
+	bootstrapFiles, err := filepath.Glob(filepath.Join(manifestsDir, "Machine\\-openshift\\-cluster\\-api\\-guests\\-*\\-bootstrap.yaml"))
+	if err != nil {
+		return "", fmt.Errorf("failed to list bootstrap manifests: %w", err)
+	}
+	logrus.Debugf("bootstrap manifests found: %v", bootstrapFiles)
+
+	if len(bootstrapFiles) != 1 {
+		return "", fmt.Errorf("wrong number of bootstrap manifests found: %v. Expected exactly one", bootstrapFiles)
+	}
+	addrs, err := extractIPAddress(bootstrapFiles[0])
+	if err != nil {
+		return "", fmt.Errorf("failed to extract IP address for bootstrap: %w", err)
+	}
+	logrus.Debugf("found bootstrap address: %s", addrs)
+	return prioritizeIPv4(config, addrs), nil
+}
+
 // IgnitionSecret provides the basic formatting for creating the
 // ignition secret.
 func IgnitionSecret(ign []byte, infraID, role string) *corev1.Secret {

pkg/infrastructure/clusterapi/types.go

@@ -20,11 +20,14 @@ 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
+	// PublicGatherEndpoint returns how the cloud platform expects the installer
+	// to connect to the bootstrap node for log gathering. CAPI providers are not
+	// consistent in how public IP addresses are represented in the machine status.
+	// Furthermore, Azure cannot attach a public IP to the bootstrap node, so SSH
+	// must be performed through the API load balancer.
+	// When a platform returns ExternalIP, the installer will require an ExternalIP
+	// to be present in the status, before it declares the machine ready.
+	PublicGatherEndpoint() GatherEndpoint
 }
 
 // PreProvider defines the PreProvision hook, which is called prior to
@@ -126,3 +129,18 @@ type Timeouts interface {
 	// When waiting for the machines to provision.
 	ProvisionTimeout() time.Duration
 }
+
+// GatherEndpoint represents the valid values for connecting to the bootstrap nude
+// in a public cluster to gather logs.
+type GatherEndpoint string
+
+const (
+	// ExternalIP indicates that the machine status will include an ExternalIP that can be used for gather.
+	ExternalIP GatherEndpoint = "ExternalIP"
+
+	// InternalIP indicates that the machine status will only include InternalIPs.
+	InternalIP GatherEndpoint = "InternalIP"
+
+	// APILoadBalancer indicates that gather bootstrap should connect to the API load balancer.
+	APILoadBalancer GatherEndpoint = "APILoadBalancer"
+)

pkg/infrastructure/gcp/clusterapi/clusterapi.go

@@ -37,9 +37,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should wait for an ExternalIP
+// in the status and use that when gathering bootstrap log bundles.
+func (Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.ExternalIP }
 
 // 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

pkg/infrastructure/ibmcloud/clusterapi/clusterapi.go

@@ -18,9 +18,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should NOT wait for an ExternalIP
+// in the status when declaring machines ready.
+func (Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.InternalIP }
 
 // PreProvision creates the IBM Cloud objects required prior to running capibmcloud.
 func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionInput) error {

pkg/infrastructure/nutanix/clusterapi/clusterapi.go

@@ -26,9 +26,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should NOT wait for an ExternalIP
+// in the status when declaring machines ready.
+func (Provider) PublicGatherEndpoint() infracapi.GatherEndpoint { return infracapi.InternalIP }
 
 // PreProvision creates the resources required prior to running capi nutanix controller.
 func (p Provider) PreProvision(ctx context.Context, in infracapi.PreProvisionInput) error {

pkg/infrastructure/openstack/clusterapi/clusterapi.go

@@ -31,9 +31,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should NOT wait for an ExternalIP
+// in the status when declaring machines ready.
+func (Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.InternalIP }
 
 var _ clusterapi.PreProvider = Provider{}
 

pkg/infrastructure/powervs/clusterapi/powervs.go

@@ -39,9 +39,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should NOT wait for an ExternalIP
+// in the status when declaring machines ready.
+func (Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.InternalIP }
 
 func leftInContext(ctx context.Context) time.Duration {
 	deadline, ok := ctx.Deadline()

pkg/infrastructure/vsphere/clusterapi/clusterapi.go

@@ -28,9 +28,9 @@ 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 }
+// PublicGatherEndpoint indicates that machine ready checks should NOT wait for an ExternalIP
+// in the status when declaring machines ready.
+func (Provider) PublicGatherEndpoint() clusterapi.GatherEndpoint { return clusterapi.InternalIP }
 
 func initializeFoldersAndTemplates(ctx context.Context, cachedImage string, failureDomain vsphere.FailureDomain, session *session.Session, diskType vsphere.DiskType, clusterID, tagID string) error {
 	finder := session.Finder