Merge pull request #15620 from johngmyers/nodeup-model

Remove more references to ClusterSpec from nodeup
kubernetes · Jul 11, 2023 · 3ce0921 · 3ce0921
2 parents f831255 + a56e8eb
commit 3ce0921
Show file tree

Hide file tree

Showing 11 changed files with 44 additions and 34 deletions.
diff --git a/cmd/kops/integration_test.go b/cmd/kops/integration_test.go
@@ -1503,7 +1503,7 @@ func (i *integrationTest) setupCluster(t *testing.T, ctx context.Context, inputY
 			secondaryCertificate: "-----BEGIN CERTIFICATE-----\nMIIBfDCCASagAwIBAgIMFo+b23acX0hZEkbkMA0GCSqGSIb3DQEBCwUAMB8xHTAb\nBgNVBAMTFGV0Y2QtcGVlcnMtY2EtY2lsaXVtMB4XDTIxMDcwNTIwMjIzN1oXDTMx\nMDcwNTIwMjIzN1owHzEdMBsGA1UEAxMUZXRjZC1wZWVycy1jYS1jaWxpdW0wXDAN\nBgkqhkiG9w0BAQEFAANLADBIAkEAw3T2pyEOgBPBKwofuILLokPxAFplVzdu540f\noREJ4iVqiroUlsz1G90mEwmqR+B7/0kt70ve9i5Z6E7Qz2nQaQIDAQABo0IwQDAO\nBgNVHQ8BAf8EBAMCAQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQU0hyEvGir\n2ucsJrojyZaDBIb8JLAwDQYJKoZIhvcNAQELBQADQQA9vQylgkvgROIMspzOlbZr\nZwsTAzp9J2ZxZL06AQ9iWzpvIw/H3oClV63q6zN2aHtpBTkhUOSX3Q4L/X/0MOkj\n-----END CERTIFICATE-----",
 		})
 	}
-	if !model.UseKopsControllerForNodeBootstrap(cluster) {
+	if !model.UseKopsControllerForNodeBootstrap(cluster.Spec.GetCloudProvider()) {
 		storeKeyset(t, ctx, keyStore, "kubelet", &testingKeyset{
 			primaryKey:           "-----BEGIN RSA PRIVATE KEY-----\nMIIBOgIBAAJBAM6BUO6Gjjskn8s87GdJB8QPpNTx949t5Z/GgQpLVCapj741c1//\nvyH6JPsyqFUVy+lsBXQHSdCz2awMhKd9x5kCAwEAAQJARozbj4Ic2Yvbo92+jlLe\n+la146J/B1tuVbXFpDS0HTi3W94fVfu6R7FR9um1te1hzBAr6I4RqXxBAvipzG9P\n4QIhAPUg1AV/uyzKxELhVNKysAqvz1oLx2NeAh3DewRQn2MNAiEA16n2q69vFDvd\nnoCi2jwfR9/VyuMjloJElRyG1hoqg70CIQDkH/QRVgkcq2uxDkFBgLgiifF/zJx3\n1mJDzsuqfVmH9QIgEP/2z8W+bcviRlJBhA5lMNc2FQ4eigiuu0pKXqolW8kCIBy/\n27C5grBlEqjw1taSKqoSnylUW6SL8N8UR0MJU5up\n-----END RSA PRIVATE KEY-----",
 			primaryCertificate:   "-----BEGIN CERTIFICATE-----\nMIIBkzCCAT2gAwIBAgIMFpL6CzllQiBcgTbiMA0GCSqGSIb3DQEBCwUAMBgxFjAU\nBgNVBAMTDWt1YmVybmV0ZXMtY2EwHhcNMjEwNzE2MTk0MjIxWhcNMzEwNzE2MTk0\nMjIxWjApMRUwEwYDVQQKEwxzeXN0ZW06bm9kZXMxEDAOBgNVBAMTB2t1YmVsZXQw\nXDANBgkqhkiG9w0BAQEFAANLADBIAkEAzoFQ7oaOOySfyzzsZ0kHxA+k1PH3j23l\nn8aBCktUJqmPvjVzX/+/Ifok+zKoVRXL6WwFdAdJ0LPZrAyEp33HmQIDAQABo1Yw\nVDAOBgNVHQ8BAf8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/\nBAIwADAfBgNVHSMEGDAWgBTRt81Y03C5ScA7CePyvQ1eyqIVADANBgkqhkiG9w0B\nAQsFAANBAGOPYAM8wEDpRs4Sa+UxSRNM5xt2a0ctNqLxYbN0gsoTXY3vEFb06qLH\npgBJgBLXG8siOEhyEhsFiXSw4klQ/y8=\n-----END CERTIFICATE-----",

diff --git a/nodeup/pkg/model/context.go b/nodeup/pkg/model/context.go
@@ -394,14 +394,18 @@ func (c *NodeupModelContext) UseVolumeMounts() bool {
 
 // UseKopsControllerForNodeBootstrap checks if nodeup should use kops-controller to bootstrap.
 func (c *NodeupModelContext) UseKopsControllerForNodeBootstrap() bool {
-	return model.UseKopsControllerForNodeBootstrap(c.Cluster)
+	return model.UseKopsControllerForNodeBootstrap(c.CloudProvider())
 }
 
 // UseChallengeCallback is true if we should use a callback challenge during node provisioning with kops-controller.
 func (c *NodeupModelContext) UseChallengeCallback(cloudProvider kops.CloudProviderID) bool {
 	return model.UseChallengeCallback(cloudProvider)
 }
 
+func (c *NodeupModelContext) UseExternalECRCredentialsProvider() bool {
+	return model.UseExternalECRCredentialsProvider(c.kubernetesVersion, c.CloudProvider())
+}
+
 // UsesSecondaryIP checks if the CNI in use attaches secondary interfaces to the host.
 func (c *NodeupModelContext) UsesSecondaryIP() bool {
 	return (c.NodeupConfig.Networking.CNI != nil && c.NodeupConfig.Networking.CNI.UsesSecondaryIP) ||

diff --git a/nodeup/pkg/model/kubelet.go b/nodeup/pkg/model/kubelet.go
@@ -158,7 +158,7 @@ func (b *KubeletBuilder) Build(c *fi.NodeupModelBuilderContext) error {
 		return err
 	}
 
-	if b.Cluster.UsesExternalECRCredentialsProvider() {
+	if b.UseExternalECRCredentialsProvider() {
 		if err := b.addECRCP(c); err != nil {
 			return fmt.Errorf("failed to add ECR credential provider: %w", err)
 		}
@@ -346,7 +346,7 @@ func (b *KubeletBuilder) buildSystemdEnvironmentFile(kubeletConfig *kops.Kubelet
 
 	flags += " --config=" + kubeletConfigFilePath
 
-	if b.Cluster.UsesExternalECRCredentialsProvider() {
+	if b.UseExternalECRCredentialsProvider() {
 		flags += " --image-credential-provider-config=" + credentialProviderConfigFilePath
 		flags += " --image-credential-provider-bin-dir=" + b.binaryPath()
 	}

diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go
@@ -909,10 +909,6 @@ func (c *Cluster) UsesNoneDNS() bool {
 	return false
 }
 
-func (c *Cluster) UsesExternalECRCredentialsProvider() bool {
-	return c.IsKubernetesGTE("1.27") && c.Spec.GetCloudProvider() == CloudProviderAWS
-}
-
 func (c *Cluster) APIInternalName() string {
 	return "api.internal." + c.ObjectMeta.Name
 }

diff --git a/pkg/apis/kops/model/features.go b/pkg/apis/kops/model/features.go
@@ -17,27 +17,14 @@ limitations under the License.
 package model
 
 import (
+	"github.com/blang/semver/v4"
 	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/apis/kops/util"
 )
 
 // UseKopsControllerForNodeBootstrap is true if nodeup should use kops-controller for bootstrapping.
-func UseKopsControllerForNodeBootstrap(cluster *kops.Cluster) bool {
-	switch cluster.Spec.GetCloudProvider() {
-	case kops.CloudProviderAWS:
-		return true
-	case kops.CloudProviderGCE:
-		return true
-	case kops.CloudProviderHetzner:
-		return true
-	case kops.CloudProviderOpenstack:
-		return true
-	case kops.CloudProviderDO:
-		return true
-	case kops.CloudProviderScaleway:
-		return true
-	default:
-		return false
-	}
+func UseKopsControllerForNodeBootstrap(cloudProvider kops.CloudProviderID) bool {
+	return cloudProvider != kops.CloudProviderAzure
 }
 
 // UseChallengeCallback is true if we should use a callback challenge during node provisioning with kops-controller.
@@ -67,7 +54,7 @@ func UseKopsControllerForNodeConfig(cluster *kops.Cluster) bool {
 			return false
 		}
 	}
-	return UseKopsControllerForNodeBootstrap(cluster)
+	return UseKopsControllerForNodeBootstrap(cluster.Spec.GetCloudProvider())
 }
 
 // UseCiliumEtcd is true if we are using the Cilium etcd cluster.
@@ -84,3 +71,7 @@ func UseCiliumEtcd(cluster *kops.Cluster) bool {
 
 	return false
 }
+
+func UseExternalECRCredentialsProvider(k8sVersion semver.Version, cloudProvider kops.CloudProviderID) bool {
+	return util.IsKubernetesGTE("1.27", k8sVersion) && cloudProvider == kops.CloudProviderAWS
+}
diff --git a/pkg/model/bootstrapscript.go b/pkg/model/bootstrapscript.go
@@ -255,12 +255,12 @@ func (b *BootstrapScriptBuilder) ResourceNodeUp(c *fi.CloudupModelBuilderContext
 		}
 	}
 
-	if model.UseCiliumEtcd(b.Cluster) && !model.UseKopsControllerForNodeBootstrap(b.Cluster) {
+	if model.UseCiliumEtcd(b.Cluster) && !model.UseKopsControllerForNodeBootstrap(b.Cluster.Spec.GetCloudProvider()) {
 		keypairs = append(keypairs, "etcd-client-cilium")
 	}
 	if ig.HasAPIServer() {
 		keypairs = append(keypairs, "apiserver-aggregator-ca", "service-account", "etcd-clients-ca")
-	} else if !model.UseKopsControllerForNodeBootstrap(b.Cluster) {
+	} else if !model.UseKopsControllerForNodeBootstrap(b.Cluster.Spec.GetCloudProvider()) {
 		keypairs = append(keypairs, "kubelet", "kube-proxy")
 		if b.Cluster.Spec.Networking.KubeRouter != nil {
 			keypairs = append(keypairs, "kube-router")

diff --git a/pkg/model/context.go b/pkg/model/context.go
@@ -253,7 +253,7 @@ func (b *KopsModelContext) CloudTags(name string, shared bool) map[string]string
 
 // UseKopsControllerForNodeBootstrap checks if nodeup should use kops-controller to bootstrap.
 func (b *KopsModelContext) UseKopsControllerForNodeBootstrap() bool {
-	return model.UseKopsControllerForNodeBootstrap(b.Cluster)
+	return model.UseKopsControllerForNodeBootstrap(b.Cluster.Spec.GetCloudProvider())
 }
 
 // UseBootstrapTokens checks if bootstrap tokens are enabled

diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go
@@ -702,7 +702,7 @@ func ReadableStatePaths(cluster *kops.Cluster, role Subject) ([]string, error) {
 				"/igconfig/node/*",
 			)
 		}
-		if !model.UseKopsControllerForNodeBootstrap(cluster) {
+		if !model.UseKopsControllerForNodeBootstrap(cluster.Spec.GetCloudProvider()) {
 			paths = append(paths,
 				"/secrets/dockerconfig",
 				"/pki/private/kube-proxy/*",

diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go
@@ -1054,7 +1054,8 @@ func (c *ApplyClusterCmd) addFileAssets(assetBuilder *assets.AssetBuilder) error
 			c.Assets[arch] = append(c.Assets[arch], mirrors.BuildMirroredAsset(u, hash))
 		}
 
-		if c.Cluster.UsesExternalECRCredentialsProvider() {
+		kubernetesVersion, _ := util.ParseKubernetesVersion(c.Cluster.Spec.KubernetesVersion)
+		if apiModel.UseExternalECRCredentialsProvider(*kubernetesVersion, c.Cluster.Spec.GetCloudProvider()) {
 			binaryLocation := c.Cluster.Spec.CloudProvider.AWS.BinariesLocation
 			if binaryLocation == nil {
 				binaryLocation = fi.PtrTo("https://artifacts.k8s.io/binaries/cloud-provider-aws/v1.27.1")
@@ -1354,7 +1355,7 @@ func (n *nodeUpConfigBuilder) BuildConfig(ig *kops.InstanceGroup, apiserverAddit
 			return nil, nil, err
 		}
 		if keysets["etcd-clients-ca-cilium"] != nil {
-			if err := loadCertificates(keysets, "etcd-clients-ca-cilium", config, hasAPIServer || apiModel.UseKopsControllerForNodeBootstrap(n.cluster)); err != nil {
+			if err := loadCertificates(keysets, "etcd-clients-ca-cilium", config, hasAPIServer || apiModel.UseKopsControllerForNodeBootstrap(n.cluster.Spec.GetCloudProvider())); err != nil {
 				return nil, nil, err
 			}
 		}

diff --git a/upup/pkg/fi/cloudup/dns.go b/upup/pkg/fi/cloudup/dns.go
@@ -276,7 +276,7 @@ func buildPrecreateDNSHostnames(cluster *kops.Cluster) []recordKey {
 		})
 	}
 
-	if apimodel.UseKopsControllerForNodeBootstrap(cluster) {
+	if apimodel.UseKopsControllerForNodeBootstrap(cluster.Spec.GetCloudProvider()) {
 		name := "kops-controller.internal." + cluster.ObjectMeta.Name
 		recordKeys = append(recordKeys, recordKey{
 			hostname: name,

diff --git a/upup/pkg/fi/cloudup/dns_test.go b/upup/pkg/fi/cloudup/dns_test.go
@@ -31,7 +31,13 @@ func TestPrecreateDNSNames(t *testing.T) {
 		expected []recordKey
 	}{
 		{
-			cluster: &kops.Cluster{},
+			cluster: &kops.Cluster{
+				Spec: kops.ClusterSpec{
+					CloudProvider: kops.CloudProviderSpec{
+						Azure: &kops.AzureSpec{},
+					},
+				},
+			},
 			expected: []recordKey{
 				{"api.cluster1.example.com", rrstype.A},
 				{"api.internal.cluster1.example.com", rrstype.A},
@@ -40,6 +46,9 @@ func TestPrecreateDNSNames(t *testing.T) {
 		{
 			cluster: &kops.Cluster{
 				Spec: kops.ClusterSpec{
+					CloudProvider: kops.CloudProviderSpec{
+						Azure: &kops.AzureSpec{},
+					},
 					Networking: kops.NetworkingSpec{
 						NonMasqueradeCIDR: "::/0",
 					},
@@ -57,6 +66,9 @@ func TestPrecreateDNSNames(t *testing.T) {
 					API: kops.APISpec{
 						LoadBalancer: &kops.LoadBalancerAccessSpec{},
 					},
+					CloudProvider: kops.CloudProviderSpec{
+						Azure: &kops.AzureSpec{},
+					},
 				},
 			},
 			expected: []recordKey{
@@ -69,6 +81,9 @@ func TestPrecreateDNSNames(t *testing.T) {
 					API: kops.APISpec{
 						LoadBalancer: &kops.LoadBalancerAccessSpec{},
 					},
+					CloudProvider: kops.CloudProviderSpec{
+						Azure: &kops.AzureSpec{},
+					},
 					Networking: kops.NetworkingSpec{
 						NonMasqueradeCIDR: "::/0",
 					},
@@ -86,6 +101,9 @@ func TestPrecreateDNSNames(t *testing.T) {
 							UseForInternalAPI: true,
 						},
 					},
+					CloudProvider: kops.CloudProviderSpec{
+						Azure: &kops.AzureSpec{},
+					},
 				},
 			},
 			expected: nil,