Merge pull request #15829 from justinsb/refactor_forapiserver

Refactor: Replace ForAPIServer with WellKnownServices

pkg/commands/toolbox_enroll.go

@@ -47,7 +47,9 @@ import (
 	"k8s.io/kops/pkg/client/simple"
 	"k8s.io/kops/pkg/commands/commandutils"
 	"k8s.io/kops/pkg/featureflag"
+	"k8s.io/kops/pkg/model"
 	"k8s.io/kops/pkg/model/resources"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
 	"k8s.io/kops/util/pkg/architectures"
@@ -104,7 +106,8 @@ func RunToolboxEnroll(ctx context.Context, f commandutils.Factory, out io.Writer
 		return err
 	}
 
-	apiserverAdditionalIPs := []string{}
+	wellKnownAddresses := make(model.WellKnownAddresses)
+
 	{
 		ingresses, err := cloud.GetApiIngressStatus(cluster)
 		if err != nil {
@@ -117,17 +120,21 @@ func RunToolboxEnroll(ctx context.Context, f commandutils.Factory, out io.Writer
 			// 	apiserverAdditionalIPs = append(apiserverAdditionalIPs, ingress.Hostname)
 			// }
 			if ingress.IP != "" {
-				apiserverAdditionalIPs = append(apiserverAdditionalIPs, ingress.IP)
+				wellKnownAddresses[wellknownservices.KubeAPIServer] = append(wellKnownAddresses[wellknownservices.KubeAPIServer], ingress.IP)
 			}
 		}
 	}
 
-	if len(apiserverAdditionalIPs) == 0 {
+	if len(wellKnownAddresses[wellknownservices.KubeAPIServer]) == 0 {
 		// TODO: Should we support DNS?
-		return fmt.Errorf("unable to determine IP address for kops-controller")
+		return fmt.Errorf("unable to determine IP address for kube-apiserver")
+	}
+
+	for k := range wellKnownAddresses {
+		sort.Strings(wellKnownAddresses[k])
 	}
 
-	scriptBytes, err := buildBootstrapData(ctx, clientset, cluster, ig, apiserverAdditionalIPs)
+	scriptBytes, err := buildBootstrapData(ctx, clientset, cluster, ig, wellKnownAddresses)
 	if err != nil {
 		return err
 	}
@@ -390,7 +397,7 @@ func (s *SSHHost) getHostname(ctx context.Context) (string, error) {
 	return hostname, nil
 }
 
-func buildBootstrapData(ctx context.Context, clientset simple.Clientset, cluster *kops.Cluster, ig *kops.InstanceGroup, apiserverAdditionalIPs []string) ([]byte, error) {
+func buildBootstrapData(ctx context.Context, clientset simple.Clientset, cluster *kops.Cluster, ig *kops.InstanceGroup, wellknownAddresses model.WellKnownAddresses) ([]byte, error) {
 	if cluster.Spec.KubeAPIServer == nil {
 		cluster.Spec.KubeAPIServer = &kops.KubeAPIServerConfig{}
 	}
@@ -451,7 +458,7 @@ func buildBootstrapData(ctx context.Context, clientset simple.Clientset, cluster
 		keysets[keyName] = keyset
 	}
 
-	_, bootConfig, err := configBuilder.BuildConfig(ig, apiserverAdditionalIPs, keysets)
+	_, bootConfig, err := configBuilder.BuildConfig(ig, wellknownAddresses, keysets)
 	if err != nil {
 		return nil, err
 	}

pkg/model/awsmodel/api_loadbalancer.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/wellknownports"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
 )
@@ -187,10 +188,10 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 			Listeners:      nlbListeners,
 			TargetGroups:   make([]*awstasks.TargetGroup, 0),
 
-			Tags:         tags,
-			ForAPIServer: true,
-			VPC:          b.LinkToVPC(),
-			Type:         fi.PtrTo("network"),
+			Tags:              tags,
+			WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KubeAPIServer},
+			VPC:               b.LinkToVPC(),
+			Type:              fi.PtrTo("network"),
 		}
 
 		clb = &awstasks.ClassicLoadBalancer{
@@ -222,8 +223,8 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 				Timeout: fi.PtrTo(int64(300)),
 			},
 
-			Tags:         tags,
-			ForAPIServer: true,
+			Tags:              tags,
+			WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KubeAPIServer},
 		}
 
 		if b.Cluster.UsesNoneDNS() {
@@ -536,6 +537,9 @@ func (b *APILoadBalancerBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 				ToPort:        fi.PtrTo(int64(4)),
 			})
 			if b.Cluster.UsesNoneDNS() {
+				nlb.WellKnownServices = append(nlb.WellKnownServices, wellknownservices.KopsController)
+				clb.WellKnownServices = append(clb.WellKnownServices, wellknownservices.KopsController)
+
 				c.AddTask(&awstasks.SecurityGroupRule{
 					Name:          fi.PtrTo(fmt.Sprintf("kops-controller-elb-to-cp%s", suffix)),
 					Lifecycle:     b.SecurityLifecycle,

pkg/model/bootstrapscript.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/nodeup"
 	"k8s.io/kops/pkg/model/resources"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 	"k8s.io/kops/upup/pkg/fi/cloudup/scaleway"
@@ -40,9 +41,12 @@ import (
 )
 
 type NodeUpConfigBuilder interface {
-	BuildConfig(ig *kops.InstanceGroup, apiserverAdditionalIPs []string, keysets map[string]*fi.Keyset) (*nodeup.Config, *nodeup.BootConfig, error)
+	BuildConfig(ig *kops.InstanceGroup, wellKnownAddresses WellKnownAddresses, keysets map[string]*fi.Keyset) (*nodeup.Config, *nodeup.BootConfig, error)
 }
 
+// WellKnownAddresses holds known addresses for well-known services
+type WellKnownAddresses map[wellknownservices.WellKnownService][]string
+
 // BootstrapScriptBuilder creates the bootstrap script
 type BootstrapScriptBuilder struct {
 	*KopsModelContext
@@ -58,8 +62,9 @@ type BootstrapScript struct {
 	ig        *kops.InstanceGroup
 	builder   *BootstrapScriptBuilder
 	resource  fi.CloudupTaskDependentResource
-	// alternateNameTasks are tasks that contribute api-server IP addresses.
-	alternateNameTasks []fi.HasAddress
+
+	// hasAddressTasks holds fi.HasAddress tasks, that contribute well-known services.
+	hasAddressTasks []fi.HasAddress
 
 	// caTasks hold the CA tasks, for dependency analysis.
 	caTasks map[string]*fitasks.Keypair
@@ -76,9 +81,9 @@ var (
 
 // kubeEnv returns the boot config for the instance group
 func (b *BootstrapScript) kubeEnv(ig *kops.InstanceGroup, c *fi.CloudupContext) (*nodeup.BootConfig, error) {
-	var alternateNames []string
+	wellKnownAddresses := make(WellKnownAddresses)
 
-	for _, hasAddress := range b.alternateNameTasks {
+	for _, hasAddress := range b.hasAddressTasks {
 		addresses, err := hasAddress.FindAddresses(c)
 		if err != nil {
 			return nil, fmt.Errorf("error finding address for %v: %v", hasAddress, err)
@@ -88,13 +93,17 @@ func (b *BootstrapScript) kubeEnv(ig *kops.InstanceGroup, c *fi.CloudupContext)
 			klog.V(2).Infof("Task did not have an address: %v", hasAddress)
 			continue
 		}
-		for _, address := range addresses {
-			klog.V(8).Infof("Resolved alternateName %q for %q", address, hasAddress)
-			alternateNames = append(alternateNames, address)
+
+		klog.V(8).Infof("Resolved alternateNames %q for %q", addresses, hasAddress)
+
+		for _, wellKnownService := range hasAddress.GetWellKnownServices() {
+			wellKnownAddresses[wellKnownService] = append(wellKnownAddresses[wellKnownService], addresses...)
 		}
 	}
 
-	sort.Strings(alternateNames)
+	for k := range wellKnownAddresses {
+		sort.Strings(wellKnownAddresses[k])
+	}
 
 	keysets := make(map[string]*fi.Keyset)
 	for _, caTask := range b.caTasks {
@@ -105,7 +114,7 @@ func (b *BootstrapScript) kubeEnv(ig *kops.InstanceGroup, c *fi.CloudupContext)
 		}
 		keysets[name] = keyset
 	}
-	config, bootConfig, err := b.builder.NodeUpConfigBuilder.BuildConfig(ig, alternateNames, keysets)
+	config, bootConfig, err := b.builder.NodeUpConfigBuilder.BuildConfig(ig, wellKnownAddresses, keysets)
 	if err != nil {
 		return nil, err
 	}
@@ -288,9 +297,9 @@ func (b *BootstrapScript) GetDependencies(tasks map[string]fi.CloudupTask) []fi.
 	var deps []fi.CloudupTask
 
 	for _, task := range tasks {
-		if hasAddress, ok := task.(fi.HasAddress); ok && hasAddress.IsForAPIServer() {
+		if hasAddress, ok := task.(fi.HasAddress); ok && len(hasAddress.GetWellKnownServices()) > 0 {
 			deps = append(deps, task)
-			b.alternateNameTasks = append(b.alternateNameTasks, hasAddress)
+			b.hasAddressTasks = append(b.hasAddressTasks, hasAddress)
 		}
 	}
 

pkg/model/bootstrapscript_test.go

@@ -72,7 +72,7 @@ type nodeupConfigBuilder struct {
 	cluster *kops.Cluster
 }
 
-func (n *nodeupConfigBuilder) BuildConfig(ig *kops.InstanceGroup, apiserverAdditionalIPs []string, keysets map[string]*fi.Keyset) (*nodeup.Config, *nodeup.BootConfig, error) {
+func (n *nodeupConfigBuilder) BuildConfig(ig *kops.InstanceGroup, wellKnownAddresses WellKnownAddresses, keysets map[string]*fi.Keyset) (*nodeup.Config, *nodeup.BootConfig, error) {
 	config, bootConfig := nodeup.NewConfig(n.cluster, ig)
 	return config, bootConfig, nil
 }

pkg/model/domodel/api_loadbalancer.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/do"
 	"k8s.io/kops/upup/pkg/fi/cloudup/dotasks"
@@ -60,10 +61,11 @@ func (b *APILoadBalancerModelBuilder) Build(c *fi.CloudupModelBuilderContext) er
 
 	// Create LoadBalancer for API LB
 	loadbalancer := &dotasks.LoadBalancer{
-		Name:       fi.PtrTo(loadbalancerName),
-		Region:     fi.PtrTo(b.Cluster.Spec.Networking.Subnets[0].Region),
-		DropletTag: fi.PtrTo(clusterMasterTag),
-		Lifecycle:  b.Lifecycle,
+		Name:              fi.PtrTo(loadbalancerName),
+		Region:            fi.PtrTo(b.Cluster.Spec.Networking.Subnets[0].Region),
+		DropletTag:        fi.PtrTo(clusterMasterTag),
+		Lifecycle:         b.Lifecycle,
+		WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KopsController, wellknownservices.KubeAPIServer},
 	}
 
 	if b.Cluster.Spec.Networking.NetworkID != "" {
@@ -76,11 +78,5 @@ func (b *APILoadBalancerModelBuilder) Build(c *fi.CloudupModelBuilderContext) er
 
 	c.AddTask(loadbalancer)
 
-	// Ensure the LB hostname is included in the TLS certificate,
-	// if we're not going to use an alias for it
-	if b.Cluster.UsesLegacyGossip() || b.Cluster.UsesPrivateDNS() || b.Cluster.UsesNoneDNS() {
-		loadbalancer.ForAPIServer = true
-	}
-
 	return nil
 }

pkg/model/gcemodel/api_loadbalancer.go

@@ -23,6 +23,7 @@ import (
 	"golang.org/x/exp/slices"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/wellknownports"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/gce"
 	"k8s.io/kops/upup/pkg/fi/cloudup/gcetasks"
@@ -64,9 +65,10 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext
 	c.AddTask(poolHealthCheck)
 
 	ipAddress := &gcetasks.Address{
-		Name:         s(b.NameForIPAddress("api")),
-		ForAPIServer: true,
-		Lifecycle:    b.Lifecycle,
+		Name: s(b.NameForIPAddress("api")),
+
+		Lifecycle:         b.Lifecycle,
+		WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KubeAPIServer},
 	}
 	c.AddTask(ipAddress)
 
@@ -86,6 +88,8 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext
 		},
 	})
 	if b.Cluster.UsesNoneDNS() {
+		ipAddress.WellKnownServices = append(ipAddress.WellKnownServices, wellknownservices.KopsController)
+
 		c.AddTask(&gcetasks.ForwardingRule{
 			Name:                s(b.NameForForwardingRule("kops-controller")),
 			Lifecycle:           b.Lifecycle,
@@ -203,8 +207,9 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte
 			IPAddressType: s("INTERNAL"),
 			Purpose:       s("SHARED_LOADBALANCER_VIP"),
 			Subnetwork:    subnet,
-			ForAPIServer:  true,
-			Lifecycle:     b.Lifecycle,
+
+			WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KubeAPIServer},
+			Lifecycle:         b.Lifecycle,
 		}
 		c.AddTask(ipAddress)
 
@@ -224,6 +229,8 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte
 			},
 		})
 		if b.Cluster.UsesNoneDNS() {
+			ipAddress.WellKnownServices = append(ipAddress.WellKnownServices, wellknownservices.KopsController)
+
 			c.AddTask(&gcetasks.ForwardingRule{
 				Name:                s(b.NameForForwardingRule("kops-controller-" + sn.Name)),
 				Lifecycle:           b.Lifecycle,

pkg/model/hetznermodel/loadbalancer.go

@@ -23,6 +23,7 @@ import (
 	"github.com/hetznercloud/hcloud-go/hcloud"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/wellknownports"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/hetzner"
 	"k8s.io/kops/upup/pkg/fi/cloudup/hetznertasks"
@@ -63,6 +64,8 @@ func (b *LoadBalancerModelBuilder) Build(c *fi.CloudupModelBuilderContext) error
 		Labels: map[string]string{
 			hetzner.TagKubernetesClusterName: b.ClusterName(),
 		},
+
+		WellKnownServices: []wellknownservices.WellKnownService{wellknownservices.KubeAPIServer, wellknownservices.KopsController},
 	}
 
 	c.AddTask(&loadbalancer)

pkg/model/openstackmodel/servergroup.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/kops/pkg/model"
 	"k8s.io/kops/pkg/truncate"
 	"k8s.io/kops/pkg/wellknownports"
+	"k8s.io/kops/pkg/wellknownservices"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/openstack"
 	"k8s.io/kops/upup/pkg/fi/cloudup/openstacktasks"
@@ -240,7 +241,9 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.CloudupModelBuilderContex
 				}
 				c.AddTask(t)
 				if ig.Spec.Role == kops.InstanceGroupRoleControlPlane {
-					b.associateFIPToKeypair(t)
+					// Ensure the floating IP is included in the TLS certificate,
+					// if we're not going to use an alias for it
+					t.WellKnownServices = append(t.WellKnownServices, wellknownservices.KubeAPIServer, wellknownservices.KopsController)
 				}
 				instanceTask.FloatingIP = t
 			}
@@ -250,12 +253,6 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.CloudupModelBuilderContex
 	return nil
 }
 
-func (b *ServerGroupModelBuilder) associateFIPToKeypair(fipTask *openstacktasks.FloatingIP) {
-	// Ensure the floating IP is included in the TLS certificate,
-	// if we're not going to use an alias for it
-	fipTask.ForAPIServer = true
-}
-
 func (b *ServerGroupModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 	clusterName := b.ClusterName()
 
@@ -340,9 +337,7 @@ func (b *ServerGroupModelBuilder) Build(c *fi.CloudupModelBuilderContext) error
 		}
 		c.AddTask(lbfipTask)
 
-		if b.Cluster.UsesLegacyGossip() || b.Cluster.UsesPrivateDNS() || b.Cluster.UsesNoneDNS() {
-			b.associateFIPToKeypair(lbfipTask)
-		}
+		lbfipTask.WellKnownServices = append(lbfipTask.WellKnownServices, wellknownservices.KubeAPIServer)
 
 		poolTask := &openstacktasks.LBPool{
 			Name:         fi.PtrTo(fmt.Sprintf("%s-https", fi.ValueOf(lbTask.Name))),

pkg/model/openstackmodel/servergroup_test.go

@@ -1548,7 +1548,7 @@ func createBuilderForCluster(cluster *kops.Cluster, instanceGroups []*kops.Insta
 
 type nodeupConfigBuilder struct{}
 
-func (n *nodeupConfigBuilder) BuildConfig(ig *kops.InstanceGroup, apiserverAdditionalIPs []string, keysets map[string]*fi.Keyset) (*nodeup.Config, *nodeup.BootConfig, error) {
+func (n *nodeupConfigBuilder) BuildConfig(ig *kops.InstanceGroup, wellKnownAddresses model.WellKnownAddresses, keysets map[string]*fi.Keyset) (*nodeup.Config, *nodeup.BootConfig, error) {
 	return &nodeup.Config{}, &nodeup.BootConfig{}, nil
 }
 

pkg/model/openstackmodel/tests/servergroup/adds-additional-security-groups.yaml

@@ -5,7 +5,6 @@ AvailabilityZone: zone-1
 ConfigDrive: false
 Flavor: blc.2-4
 FloatingIP: null
-ForAPIServer: false
 GroupName: node
 ID: null
 Image: image-node
@@ -76,6 +75,7 @@ UserData:
   task:
     Lifecycle: ""
     Name: node
+WellKnownServices: null
 ---
 Lifecycle: ""
 Name: apiserver-aggregator-ca

pkg/model/openstackmodel/tests/servergroup/adds-cloud-labels-from-ClusterSpec.yaml

@@ -5,7 +5,6 @@ AvailabilityZone: zone-1
 ConfigDrive: false
 Flavor: blc.2-4
 FloatingIP: null
-ForAPIServer: false
 GroupName: node
 ID: null
 Image: image-node
@@ -75,6 +74,7 @@ UserData:
   task:
     Lifecycle: ""
     Name: node
+WellKnownServices: null
 ---
 Lifecycle: ""
 Name: apiserver-aggregator-ca