diff --git a/pkg/apis/nodeup/config.go b/pkg/apis/nodeup/config.go index 06a4376f14..9343c80d50 100644 --- a/pkg/apis/nodeup/config.go +++ b/pkg/apis/nodeup/config.go @@ -19,8 +19,8 @@ package nodeup import ( "strings" + "github.com/aws/aws-sdk-go/aws" "k8s.io/kops/pkg/apis/kops" - "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/util/pkg/architectures" ) @@ -85,7 +85,7 @@ type Config struct { // BootConfig is the configuration for the nodeup binary that might be too big to fit in userdata. type BootConfig struct { // CloudProvider is the cloud provider in use. - CloudProvider string + CloudProvider kops.CloudProviderID // ConfigBase is the base VFS path for config objects. ConfigBase *string `json:",omitempty"` // ConfigServer holds the configuration for the configuration server. @@ -155,7 +155,7 @@ func NewConfig(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) (*Confi } bootConfig := BootConfig{ - CloudProvider: string(cluster.Spec.GetCloudProvider()), + CloudProvider: cluster.Spec.GetCloudProvider(), InstanceGroupName: instanceGroup.ObjectMeta.Name, InstanceGroupRole: role, } @@ -176,7 +176,7 @@ func NewConfig(cluster *kops.Cluster, instanceGroup *kops.InstanceGroup) (*Confi } if cluster.Spec.Networking.AmazonVPC != nil { - config.DefaultMachineType = fi.PtrTo(strings.Split(instanceGroup.Spec.MachineType, ",")[0]) + config.DefaultMachineType = aws.String(strings.Split(instanceGroup.Spec.MachineType, ",")[0]) } if UsesInstanceIDForNodeName(cluster) { diff --git a/pkg/model/bootstrapscript.go b/pkg/model/bootstrapscript.go index 5ff3b54d96..a79bd587dd 100644 --- a/pkg/model/bootstrapscript.go +++ b/pkg/model/bootstrapscript.go @@ -350,7 +350,7 @@ func (b *BootstrapScript) Run(c *fi.CloudupContext) error { { nodeupScript.EnvironmentVariables = func() (string, error) { - env, err := b.buildEnvironmentVariables(c.Cluster) + env, err := b.buildEnvironmentVariables(c.T.Cluster) if err != nil { return "", err } @@ -370,11 +370,11 @@ func (b *BootstrapScript) Run(c *fi.CloudupContext) error { } nodeupScript.ProxyEnv = func() (string, error) { - return b.createProxyEnv(c.Cluster.Spec.Networking.EgressProxy) + return b.createProxyEnv(c.T.Cluster.Spec.Networking.EgressProxy) } nodeupScript.ClusterSpec = func() (string, error) { - cs := c.Cluster.Spec + cs := c.T.Cluster.Spec spec := make(map[string]interface{}) spec["cloudConfig"] = cs.CloudConfig @@ -433,7 +433,7 @@ func (b *BootstrapScript) Run(c *fi.CloudupContext) error { // See https://github.com/kubernetes/kops/issues/10206 for details. nodeupScript.SetSysctls = setSysctls() - nodeupScript.CloudProvider = string(c.Cluster.Spec.GetCloudProvider()) + nodeupScript.CloudProvider = string(c.T.Cluster.Spec.GetCloudProvider()) nodeupScriptResource, err := nodeupScript.Build() if err != nil { diff --git a/pkg/model/bootstrapscript_test.go b/pkg/model/bootstrapscript_test.go index 1aba77ae24..a77c7276f9 100644 --- a/pkg/model/bootstrapscript_test.go +++ b/pkg/model/bootstrapscript_test.go @@ -190,7 +190,7 @@ func TestBootstrapUserData(t *testing.T) { } require.Contains(t, c.Tasks, "BootstrapScript/testIG") - err = c.Tasks["BootstrapScript/testIG"].Run(&fi.CloudupContext{Cluster: cluster}) + err = c.Tasks["BootstrapScript/testIG"].Run(&fi.CloudupContext{T: fi.CloudupSubContext{Cluster: cluster}}) require.NoError(t, err, "running task") actual, err := fi.ResourceAsString(res) diff --git a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go index c38edddaa5..3a112c0910 100644 --- a/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go +++ b/upup/pkg/fi/cloudup/awstasks/network_load_balancer.go @@ -441,7 +441,7 @@ func (e *NetworkLoadBalancer) FindAddresses(context *fi.CloudupContext) ([]strin var addresses []string cloud := context.T.Cloud.(awsup.AWSCloud) - cluster := context.Cluster + cluster := context.T.Cluster { lb, err := cloud.FindELBV2ByNameTag(e.Tags["Name"]) diff --git a/upup/pkg/fi/cloudup/awstasks/routetable.go b/upup/pkg/fi/cloudup/awstasks/routetable.go index ecb2eed50f..249cc09478 100644 --- a/upup/pkg/fi/cloudup/awstasks/routetable.go +++ b/upup/pkg/fi/cloudup/awstasks/routetable.go @@ -74,7 +74,7 @@ func (e *RouteTable) Find(c *fi.CloudupContext) (*RouteTable, error) { var filters []*ec2.Filter filters = append(filters, &ec2.Filter{ Name: aws.String("tag-key"), - Values: aws.StringSlice([]string{"kubernetes.io/cluster/" + c.Cluster.Name}), + Values: aws.StringSlice([]string{"kubernetes.io/cluster/" + c.T.Cluster.Name}), }) filters = append(filters, &ec2.Filter{ Name: aws.String("tag:" + awsup.TagNameKopsRole), diff --git a/upup/pkg/fi/cloudup/gcetasks/subnet.go b/upup/pkg/fi/cloudup/gcetasks/subnet.go index c2d26e68b1..38f854b4fb 100644 --- a/upup/pkg/fi/cloudup/gcetasks/subnet.go +++ b/upup/pkg/fi/cloudup/gcetasks/subnet.go @@ -50,7 +50,7 @@ func (e *Subnet) CompareWithID() *string { func (e *Subnet) Find(c *fi.CloudupContext) (*Subnet, error) { cloud := c.T.Cloud.(gce.GCECloud) - _, project, err := gce.ParseNameAndProjectFromNetworkID(c.Cluster.Spec.Networking.NetworkID) + _, project, err := gce.ParseNameAndProjectFromNetworkID(c.T.Cluster.Spec.Networking.NetworkID) if err != nil { return nil, fmt.Errorf("error parsing network name from cluster spec: %w", err) } else if project == "" { diff --git a/upup/pkg/fi/cloudup/hetznertasks/servergroup.go b/upup/pkg/fi/cloudup/hetznertasks/servergroup.go index 9474e227bc..4f327f8f13 100644 --- a/upup/pkg/fi/cloudup/hetznertasks/servergroup.go +++ b/upup/pkg/fi/cloudup/hetznertasks/servergroup.go @@ -59,7 +59,7 @@ func (v *ServerGroup) Find(c *fi.CloudupContext) (*ServerGroup, error) { client := cloud.ServerClient() labelSelector := []string{ - fmt.Sprintf("%s=%s", hetzner.TagKubernetesClusterName, c.Cluster.Name), + fmt.Sprintf("%s=%s", hetzner.TagKubernetesClusterName, c.T.Cluster.Name), fmt.Sprintf("%s=%s", hetzner.TagKubernetesInstanceGroup, fi.ValueOf(v.Name)), } listOptions := hcloud.ListOpts{ diff --git a/upup/pkg/fi/cloudup/openstacktasks/port_test.go b/upup/pkg/fi/cloudup/openstacktasks/port_test.go index 2f6985a285..8ef438394b 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/port_test.go +++ b/upup/pkg/fi/cloudup/openstacktasks/port_test.go @@ -368,13 +368,13 @@ func Test_Port_Find(t *testing.T) { { desc: "nothing found", context: &fi.CloudupContext{ - Cluster: &kops.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "clusterName", - }, - }, T: fi.CloudupSubContext{ Cloud: &portCloud{}, + Cluster: &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterName", + }, + }, }, }, port: &Port{ @@ -387,11 +387,6 @@ func Test_Port_Find(t *testing.T) { { desc: "port found no tags", context: &fi.CloudupContext{ - Cluster: &kops.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "clusterName", - }, - }, T: fi.CloudupSubContext{ Cloud: &portCloud{ listPorts: []ports.Port{ @@ -411,6 +406,11 @@ func Test_Port_Find(t *testing.T) { }, }, }, + Cluster: &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterName", + }, + }, }, }, port: &Port{ @@ -436,11 +436,6 @@ func Test_Port_Find(t *testing.T) { { desc: "port found with tags", context: &fi.CloudupContext{ - Cluster: &kops.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "clusterName", - }, - }, T: fi.CloudupSubContext{ Cloud: &portCloud{ listPorts: []ports.Port{ @@ -460,6 +455,11 @@ func Test_Port_Find(t *testing.T) { }, }, }, + Cluster: &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterName", + }, + }, }, }, port: &Port{ @@ -487,11 +487,6 @@ func Test_Port_Find(t *testing.T) { { desc: "multiple ports found", context: &fi.CloudupContext{ - Cluster: &kops.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "clusterName", - }, - }, T: fi.CloudupSubContext{ Cloud: &portCloud{ listPorts: []ports.Port{ @@ -507,6 +502,11 @@ func Test_Port_Find(t *testing.T) { }, }, }, + Cluster: &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterName", + }, + }, }, }, port: &Port{ @@ -519,11 +519,6 @@ func Test_Port_Find(t *testing.T) { { desc: "error listing ports", context: &fi.CloudupContext{ - Cluster: &kops.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "clusterName", - }, - }, T: fi.CloudupSubContext{ Cloud: &portCloud{ listPorts: []ports.Port{ @@ -534,6 +529,11 @@ func Test_Port_Find(t *testing.T) { }, listPortsError: fmt.Errorf("list error"), }, + Cluster: &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterName", + }, + }, }, }, port: &Port{ diff --git a/upup/pkg/fi/context.go b/upup/pkg/fi/context.go index 49ba7eb941..4aa38e2cb1 100644 --- a/upup/pkg/fi/context.go +++ b/upup/pkg/fi/context.go @@ -26,6 +26,7 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/pkg/apis/kops" + "k8s.io/kops/pkg/apis/nodeup" "k8s.io/kops/util/pkg/vfs" ) @@ -33,7 +34,6 @@ type Context[T SubContext] struct { ctx context.Context Target Target[T] - Cluster *kops.Cluster Keystore Keystore SecretStore SecretStore @@ -54,12 +54,16 @@ type SubContext interface { } type CloudupSubContext struct { - Cloud Cloud + Cloud Cloud + Cluster *kops.Cluster // TODO: Few places use this. They could instead get it from the cluster spec. ClusterConfigBase vfs.Path } type InstallSubContext struct{} -type NodeupSubContext struct{} +type NodeupSubContext struct { + BootConfig *nodeup.BootConfig + NodeupConfig *nodeup.Config +} func (c *Context[T]) Context() context.Context { return c.ctx @@ -71,10 +75,9 @@ type Warning[T SubContext] struct { Message string } -func newContext[T SubContext](ctx context.Context, target Target[T], cluster *kops.Cluster, keystore Keystore, secretStore SecretStore, checkExisting bool, sub T, tasks map[string]Task[T]) (*Context[T], error) { +func newContext[T SubContext](ctx context.Context, target Target[T], keystore Keystore, secretStore SecretStore, checkExisting bool, sub T, tasks map[string]Task[T]) (*Context[T], error) { c := &Context[T]{ ctx: ctx, - Cluster: cluster, Target: target, Keystore: keystore, SecretStore: secretStore, @@ -88,19 +91,23 @@ func newContext[T SubContext](ctx context.Context, target Target[T], cluster *ko func NewInstallContext(ctx context.Context, target InstallTarget, tasks map[string]InstallTask) (*InstallContext, error) { sub := InstallSubContext{} - return newContext[InstallSubContext](ctx, target, nil, nil, nil, true, sub, tasks) + return newContext[InstallSubContext](ctx, target, nil, nil, true, sub, tasks) } -func NewNodeupContext(ctx context.Context, target NodeupTarget, cluster *kops.Cluster, keystore Keystore, secretStore SecretStore, checkExisting bool, tasks map[string]NodeupTask) (*NodeupContext, error) { - sub := NodeupSubContext{} - return newContext[NodeupSubContext](ctx, target, cluster, keystore, secretStore, checkExisting, sub, tasks) +func NewNodeupContext(ctx context.Context, target NodeupTarget, keystore Keystore, secretStore SecretStore, checkExisting bool, bootConfig *nodeup.BootConfig, nodeupConfig *nodeup.Config, tasks map[string]NodeupTask) (*NodeupContext, error) { + sub := NodeupSubContext{ + BootConfig: bootConfig, + NodeupConfig: nodeupConfig, + } + return newContext[NodeupSubContext](ctx, target, keystore, secretStore, checkExisting, sub, tasks) } func NewCloudupContext(ctx context.Context, target CloudupTarget, cluster *kops.Cluster, cloud Cloud, keystore Keystore, secretStore SecretStore, clusterConfigBase vfs.Path, checkExisting bool, tasks map[string]CloudupTask) (*CloudupContext, error) { sub := CloudupSubContext{ Cloud: cloud, + Cluster: cluster, ClusterConfigBase: clusterConfigBase, } - return newContext[CloudupSubContext](ctx, target, cluster, keystore, secretStore, checkExisting, sub, tasks) + return newContext[CloudupSubContext](ctx, target, keystore, secretStore, checkExisting, sub, tasks) } func (c *Context[T]) AllTasks() map[string]Task[T] { diff --git a/upup/pkg/fi/fitasks/managedfile.go b/upup/pkg/fi/fitasks/managedfile.go index a148a2b486..21f21e490b 100644 --- a/upup/pkg/fi/fitasks/managedfile.go +++ b/upup/pkg/fi/fitasks/managedfile.go @@ -143,7 +143,7 @@ func (e *ManagedFile) getACL(c *fi.CloudupContext, p vfs.Path) (vfs.ACL, error) return acl, nil } - return acls.GetACL(p, c.Cluster) + return acls.GetACL(p, c.T.Cluster) } func (_ *ManagedFile) Render(c *fi.CloudupContext, a, e, changes *ManagedFile) error { diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index fba5ee004d..b8813f0958 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -181,7 +181,7 @@ func (c *NodeUpCommand) Run(out io.Writer) error { return fmt.Errorf("nodeup config hash mismatch (was %q, expected %q)", got, want) } - cloudProvider := api.CloudProviderID(bootConfig.CloudProvider) + cloudProvider := bootConfig.CloudProvider if cloudProvider == "" { cloudProvider = c.cluster.Spec.GetCloudProvider() } @@ -381,7 +381,7 @@ func (c *NodeUpCommand) Run(out io.Writer) error { return fmt.Errorf("unsupported target type %q", c.Target) } - context, err := fi.NewNodeupContext(ctx, target, c.cluster, keyStore, secretStore, checkExisting, taskMap) + context, err := fi.NewNodeupContext(ctx, target, keyStore, secretStore, checkExisting, &bootConfig, &nodeupConfig, taskMap) if err != nil { klog.Exitf("error building context: %v", err) } @@ -684,7 +684,7 @@ func loadKernelModules(context *model.NodeupModelContext) error { // getRegion queries the cloud provider for the region. func getRegion(ctx context.Context, bootConfig *nodeup.BootConfig) (string, error) { - switch api.CloudProviderID(bootConfig.CloudProvider) { + switch bootConfig.CloudProvider { case api.CloudProviderAWS: region, err := awsup.RegionFromMetadata(ctx) if err != nil { @@ -699,7 +699,7 @@ func getRegion(ctx context.Context, bootConfig *nodeup.BootConfig) (string, erro // seedRNG adds entropy to the random number generator. func seedRNG(ctx context.Context, bootConfig *nodeup.BootConfig, region string) error { - switch api.CloudProviderID(bootConfig.CloudProvider) { + switch bootConfig.CloudProvider { case api.CloudProviderAWS: config := aws.NewConfig().WithCredentialsChainVerboseErrors(true).WithRegion(region) sess, err := session.NewSession(config) @@ -735,7 +735,7 @@ func getNodeConfigFromServer(ctx context.Context, bootConfig *nodeup.BootConfig, var authenticator bootstrap.Authenticator var resolver resolver.Resolver - switch api.CloudProviderID(bootConfig.CloudProvider) { + switch bootConfig.CloudProvider { case api.CloudProviderAWS: a, err := awsup.NewAWSAuthenticator(region) if err != nil { diff --git a/upup/pkg/fi/nodeup/nodetasks/package.go b/upup/pkg/fi/nodeup/nodetasks/package.go index 1bb5e6a946..e882a5585d 100644 --- a/upup/pkg/fi/nodeup/nodetasks/package.go +++ b/upup/pkg/fi/nodeup/nodetasks/package.go @@ -197,10 +197,7 @@ func (e *Package) findDpkg(c *fi.NodeupContext) (*Package, error) { } } - // TODO: Take InstanceGroup-level overriding of the Cluster-level update policy into account - // here. Doing so requires that we make the current InstanceGroup available within Package's - // methods. - if fi.ValueOf(c.Cluster.Spec.UpdatePolicy) != kops.UpdatePolicyExternal || !installed { + if c.T.NodeupConfig.UpdatePolicy != kops.UpdatePolicyExternal || !installed { return nil, nil } @@ -248,10 +245,7 @@ func (e *Package) findYum(c *fi.NodeupContext) (*Package, error) { healthy = fi.PtrTo(true) } - // TODO: Take InstanceGroup-level overriding of the Cluster-level update policy into account - // here. Doing so requires that we make the current InstanceGroup available within Package's - // methods. - if fi.ValueOf(c.Cluster.Spec.UpdatePolicy) != kops.UpdatePolicyExternal || !installed { + if c.T.NodeupConfig.UpdatePolicy != kops.UpdatePolicyExternal || !installed { return nil, nil } diff --git a/upup/pkg/fi/nodeup/nodetasks/prefix.go b/upup/pkg/fi/nodeup/nodetasks/prefix.go index 27c6ddd282..546ccd6541 100644 --- a/upup/pkg/fi/nodeup/nodetasks/prefix.go +++ b/upup/pkg/fi/nodeup/nodetasks/prefix.go @@ -50,8 +50,8 @@ func (p *Prefix) String() string { } func (e *Prefix) Find(c *fi.NodeupContext) (*Prefix, error) { - if c.Cluster.Spec.GetCloudProvider() != kops.CloudProviderAWS { - return nil, fmt.Errorf("unsupported cloud provider: %s", c.Cluster.Spec.GetCloudProvider()) + if c.T.BootConfig.CloudProvider != kops.CloudProviderAWS { + return nil, fmt.Errorf("unsupported cloud provider: %s", c.T.BootConfig.CloudProvider) } mac, err := getInstanceMetadataFirstValue("mac")