diff --git a/pkg/cloud/aws/actuators/machine_scope.go b/pkg/cloud/aws/actuators/machine_scope.go index 061a9cdea8..cbceac18e5 100644 --- a/pkg/cloud/aws/actuators/machine_scope.go +++ b/pkg/cloud/aws/actuators/machine_scope.go @@ -97,6 +97,16 @@ func (m *MachineScope) Region() string { return m.Scope.Region() } +// GetMachine returns the machine wrapped in the scope. +func (m *MachineScope) GetMachine() *clusterv1.Machine { + return m.Machine +} + +// GetScope() returns the scope that is wrapping the machine. +func (m *MachineScope) GetScope() *Scope { + return m.Scope +} + func (m *MachineScope) storeMachineSpec(machine *clusterv1.Machine) (*clusterv1.Machine, error) { ext, err := v1alpha1.EncodeMachineSpec(m.MachineConfig) if err != nil { diff --git a/pkg/cloud/aws/services/ec2/instances.go b/pkg/cloud/aws/services/ec2/instances.go index 19e55d7d1a..a900ccc1ed 100644 --- a/pkg/cloud/aws/services/ec2/instances.go +++ b/pkg/cloud/aws/services/ec2/instances.go @@ -171,7 +171,7 @@ func (s *Service) createInstance(machine *actuators.MachineScope, bootstrapToken if bootstrapToken != "" { klog.V(2).Infof("Allowing machine %q to join control plane for cluster %q", machine.Name(), s.scope.Name()) - kubeadm.SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken, machine, &machine.MachineConfig.KubeadmConfiguration.Join) + machine.MachineConfig.KubeadmConfiguration.Join = kubeadm.SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken, machine, &machine.MachineConfig.KubeadmConfiguration.Join) kubeadm.SetControlPlaneJoinConfigurationOverrides(&machine.MachineConfig.KubeadmConfiguration.Join) joinConfigurationYAML, err := kubeadm.ConfigurationToYAML(&machine.MachineConfig.KubeadmConfiguration.Join) if err != nil { diff --git a/pkg/cloud/aws/services/kubeadm/BUILD.bazel b/pkg/cloud/aws/services/kubeadm/BUILD.bazel index 7862d93d85..51396e2932 100644 --- a/pkg/cloud/aws/services/kubeadm/BUILD.bazel +++ b/pkg/cloud/aws/services/kubeadm/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -16,7 +16,20 @@ go_library( "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library", "//vendor/k8s.io/kubernetes/cmd/kubeadm/app/util:go_default_library", + "//vendor/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1:go_default_library", "//vendor/sigs.k8s.io/cluster-api/pkg/util:go_default_library", "//vendor/sigs.k8s.io/controller-runtime/pkg/runtime/scheme:go_default_library", ], ) + +go_test( + name = "go_default_test", + srcs = ["aws_defaults_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/apis/awsprovider/v1alpha1:go_default_library", + "//pkg/cloud/aws/actuators:go_default_library", + "//vendor/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library", + "//vendor/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1:go_default_library", + ], +) diff --git a/pkg/cloud/aws/services/kubeadm/aws_defaults.go b/pkg/cloud/aws/services/kubeadm/aws_defaults.go index 9721c87401..47602f0a89 100644 --- a/pkg/cloud/aws/services/kubeadm/aws_defaults.go +++ b/pkg/cloud/aws/services/kubeadm/aws_defaults.go @@ -19,6 +19,7 @@ package kubeadm import ( "fmt" + "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" "sigs.k8s.io/cluster-api/pkg/util" "k8s.io/klog" @@ -31,16 +32,18 @@ const ( // See https://cloudinit.readthedocs.io/en/latest/topics/instancedata.html localIPV4Lookup = "{{ ds.meta_data.local_ipv4 }}" - // hostname lookup uses the instance metadata service to lookup its own hostname. - hostnameLookup = "{{ ds.meta_data.hostname }}" + // HostnameLookup uses the instance metadata service to lookup its own hostname. + HostnameLookup = "{{ ds.meta_data.hostname }}" - // containerdSocket is the expected path to containerd socket. - containerdSocket = "/var/run/containerd/containerd.sock" + // ContainerdSocket is the expected path to containerd socket. + ContainerdSocket = "/var/run/containerd/containerd.sock" - // apiServerBindPort is the default port for the kube-apiserver to bind to. - apiServerBindPort = 6443 + // APIServerBindPort is the default port for the kube-apiserver to bind to. + APIServerBindPort = 6443 - cloudProvider = "aws" + // CloudProvider is the name of the cloud provider passed to various + // kubernetes components. + CloudProvider = "aws" nodeRole = "node-role.kubernetes.io/node=" ) @@ -55,7 +58,7 @@ func SetDefaultClusterConfiguration(machine *actuators.MachineScope, base *kubea // Only set the control plane endpoint if the user hasn't specified one. if base.ControlPlaneEndpoint == "" { - base.ControlPlaneEndpoint = fmt.Sprintf("%s:%d", s.Network().APIServerELB.DNSName, apiServerBindPort) + base.ControlPlaneEndpoint = fmt.Sprintf("%s:%d", s.Network().APIServerELB.DNSName, APIServerBindPort) } // Add the control plane endpoint to the list of cert SAN base.APIServer.CertSANs = append(base.APIServer.CertSANs, localIPV4Lookup, s.Network().APIServerELB.DNSName) @@ -75,18 +78,18 @@ func SetClusterConfigurationOverrides(machine *actuators.MachineScope, base *kub if base.APIServer.ControlPlaneComponent.ExtraArgs == nil { base.APIServer.ControlPlaneComponent.ExtraArgs = map[string]string{} } - if cp, ok := base.APIServer.ControlPlaneComponent.ExtraArgs["cloud-provider"]; ok && cp != cloudProvider { - klog.Infof("Overriding cloud provider %q with required value %q", cp, cloudProvider) + if cp, ok := base.APIServer.ControlPlaneComponent.ExtraArgs["cloud-provider"]; ok && cp != CloudProvider { + klog.Infof("Overriding cloud provider %q with required value %q", cp, CloudProvider) } - base.APIServer.ControlPlaneComponent.ExtraArgs["cloud-provider"] = cloudProvider + base.APIServer.ControlPlaneComponent.ExtraArgs["cloud-provider"] = CloudProvider if base.ControllerManager.ExtraArgs == nil { base.ControllerManager.ExtraArgs = map[string]string{} } - if cp, ok := base.ControllerManager.ExtraArgs["cloud-provider"]; ok && cp != cloudProvider { - klog.Infof("Overriding cloud provider %q with required value %q", cp, cloudProvider) + if cp, ok := base.ControllerManager.ExtraArgs["cloud-provider"]; ok && cp != CloudProvider { + klog.Infof("Overriding cloud provider %q with required value %q", cp, CloudProvider) } - base.ControllerManager.ExtraArgs["cloud-provider"] = cloudProvider + base.ControllerManager.ExtraArgs["cloud-provider"] = CloudProvider // The kubeadm config clustername must match the provided name of the cluster. if base.ClusterName != "" && base.ClusterName != s.Name() { @@ -112,63 +115,70 @@ func SetInitConfigurationOverrides(base *kubeadmv1beta1.InitConfiguration) { base = &kubeadmv1beta1.InitConfiguration{} } - if base.NodeRegistration.Name != "" && base.NodeRegistration.Name != hostnameLookup { - klog.Infof("Overriding NodeRegistration name from %q to %q. The node registration needs to be dynamically generated in aws.", base.NodeRegistration.Name, hostnameLookup) + if base.NodeRegistration.Name != "" && base.NodeRegistration.Name != HostnameLookup { + klog.Infof("Overriding NodeRegistration name from %q to %q. The node registration needs to be dynamically generated in aws.", base.NodeRegistration.Name, HostnameLookup) } - base.NodeRegistration.Name = hostnameLookup + base.NodeRegistration.Name = HostnameLookup // TODO(chuckha): This may become a default instead of an override. - if base.NodeRegistration.CRISocket != "" && base.NodeRegistration.CRISocket != containerdSocket { - klog.Infof("Overriding CRISocket from %q to %q. Containerd is only supported container runtime.", base.NodeRegistration.CRISocket, containerdSocket) + if base.NodeRegistration.CRISocket != "" && base.NodeRegistration.CRISocket != ContainerdSocket { + klog.Infof("Overriding CRISocket from %q to %q. Containerd is only supported container runtime.", base.NodeRegistration.CRISocket, ContainerdSocket) } - base.NodeRegistration.CRISocket = containerdSocket + base.NodeRegistration.CRISocket = ContainerdSocket if base.NodeRegistration.KubeletExtraArgs == nil { base.NodeRegistration.KubeletExtraArgs = map[string]string{} } - if cp, ok := base.NodeRegistration.KubeletExtraArgs["cloud-provider"]; ok && cp != cloudProvider { - klog.Infof("Overriding node's cloud-provider to the required value of %q.", cloudProvider) + if cp, ok := base.NodeRegistration.KubeletExtraArgs["cloud-provider"]; ok && cp != CloudProvider { + klog.Infof("Overriding node's cloud-provider to the required value of %q.", CloudProvider) } - base.NodeRegistration.KubeletExtraArgs["cloud-provider"] = cloudProvider + base.NodeRegistration.KubeletExtraArgs["cloud-provider"] = CloudProvider +} + +// joinMachine is a local interface scoping down exactly what SetJoinNodeConfigurationOverrides needs +type joinMachine interface { + GetScope() *actuators.Scope + GetMachine() *v1alpha1.Machine } // SetJoinNodeConfigurationOverrides overrides user input for certain fields of // the kubeadm JoinConfiguration during a worker node join. -func SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken string, machine *actuators.MachineScope, base *kubeadmv1beta1.JoinConfiguration) { +func SetJoinNodeConfigurationOverrides(caCertHash, bootstrapToken string, machine joinMachine, base *kubeadmv1beta1.JoinConfiguration) kubeadmv1beta1.JoinConfiguration { if base == nil { base = &kubeadmv1beta1.JoinConfiguration{} } - s := machine.Scope + out := base.DeepCopy() - if base.Discovery.BootstrapToken == nil { - base.Discovery.BootstrapToken = &kubeadmv1beta1.BootstrapTokenDiscovery{} + if out.Discovery.BootstrapToken == nil { + out.Discovery.BootstrapToken = &kubeadmv1beta1.BootstrapTokenDiscovery{} } // TODO: should this actually be the cluster's ContolPlaneEndpoint? - base.Discovery.BootstrapToken.APIServerEndpoint = fmt.Sprintf("%s:%d", s.Network().APIServerELB.DNSName, apiServerBindPort) - base.Discovery.BootstrapToken.Token = bootstrapToken - base.Discovery.BootstrapToken.CACertHashes = append(base.Discovery.BootstrapToken.CACertHashes, caCertHash) + out.Discovery.BootstrapToken.APIServerEndpoint = fmt.Sprintf("%s:%d", machine.GetScope().Network().APIServerELB.DNSName, APIServerBindPort) + out.Discovery.BootstrapToken.Token = bootstrapToken + out.Discovery.BootstrapToken.CACertHashes = append(out.Discovery.BootstrapToken.CACertHashes, caCertHash) - if base.NodeRegistration.Name != "" && base.NodeRegistration.Name != hostnameLookup { - klog.Infof("Overriding NodeRegistration name from %q to %q. The node registration needs to be dynamically generated in aws.", base.NodeRegistration.Name, hostnameLookup) + if out.NodeRegistration.Name != "" && out.NodeRegistration.Name != HostnameLookup { + klog.Infof("Overriding NodeRegistration name from %q to %q. The node registration needs to be dynamically generated in aws.", out.NodeRegistration.Name, HostnameLookup) } - base.NodeRegistration.Name = hostnameLookup + out.NodeRegistration.Name = HostnameLookup // TODO(chuckha): This may become a default instead of an override. - if base.NodeRegistration.CRISocket != "" && base.NodeRegistration.CRISocket != containerdSocket { - klog.Infof("Overriding CRISocket from %q to %q. Containerd is only supported container runtime.", base.NodeRegistration.CRISocket, containerdSocket) + if out.NodeRegistration.CRISocket != "" && out.NodeRegistration.CRISocket != ContainerdSocket { + klog.Infof("Overriding CRISocket from %q to %q. Containerd is only supported container runtime.", out.NodeRegistration.CRISocket, ContainerdSocket) } - base.NodeRegistration.CRISocket = containerdSocket + out.NodeRegistration.CRISocket = ContainerdSocket - if base.NodeRegistration.KubeletExtraArgs == nil { - base.NodeRegistration.KubeletExtraArgs = map[string]string{} + if out.NodeRegistration.KubeletExtraArgs == nil { + out.NodeRegistration.KubeletExtraArgs = map[string]string{} } - if cp, ok := base.NodeRegistration.KubeletExtraArgs["cloud-provider"]; ok && cp != cloudProvider { - klog.Infof("Overriding node's cloud-provider to the required value of %q.", cloudProvider) + if cp, ok := out.NodeRegistration.KubeletExtraArgs["cloud-provider"]; ok && cp != CloudProvider { + klog.Infof("Overriding node's cloud-provider to the required value of %q.", CloudProvider) } - base.NodeRegistration.KubeletExtraArgs["cloud-provider"] = cloudProvider - if !util.IsControlPlaneMachine(machine.Machine) { - base.NodeRegistration.KubeletExtraArgs["node-labels"] = nodeRole + out.NodeRegistration.KubeletExtraArgs["cloud-provider"] = CloudProvider + if !util.IsControlPlaneMachine(machine.GetMachine()) { + out.NodeRegistration.KubeletExtraArgs["node-labels"] = nodeRole } + return *out } // SetControlPlaneJoinConfigurationOverrides user input for kubeadm join @@ -181,5 +191,5 @@ func SetControlPlaneJoinConfigurationOverrides(base *kubeadmv1beta1.JoinConfigur base.ControlPlane = &kubeadmv1beta1.JoinControlPlane{} } base.ControlPlane.LocalAPIEndpoint.AdvertiseAddress = localIPV4Lookup - base.ControlPlane.LocalAPIEndpoint.BindPort = apiServerBindPort + base.ControlPlane.LocalAPIEndpoint.BindPort = APIServerBindPort } diff --git a/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go b/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go new file mode 100644 index 0000000000..d0e0c3971e --- /dev/null +++ b/pkg/cloud/aws/services/kubeadm/aws_defaults_test.go @@ -0,0 +1,163 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubeadm_test + +import ( + "fmt" + "testing" + + kubeadmv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" + "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1alpha1" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/actuators" + "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/aws/services/kubeadm" + clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" +) + +type jm struct { + *actuators.Scope + *clusterv1.Machine +} + +func (j *jm) GetScope() *actuators.Scope { + return j.Scope +} +func (j *jm) GetMachine() *clusterv1.Machine { + return j.Machine +} + +func TestSetJoinNodeConfigurationBootstrapSettings(t *testing.T) { + testcases := []struct { + name string + caCertHash string + bootstrapToken string + joinMachine *jm + joinConfiguration *kubeadmv1beta1.JoinConfiguration + }{ + { + name: "test default values", + joinMachine: &jm{ + Scope: &actuators.Scope{ + ClusterStatus: &v1alpha1.AWSClusterProviderStatus{ + Network: v1alpha1.Network{ + APIServerELB: v1alpha1.ClassicELB{ + DNSName: "test.test", + }, + }, + }, + }, + Machine: &clusterv1.Machine{}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + out := kubeadm.SetJoinNodeConfigurationOverrides(tc.caCertHash, tc.bootstrapToken, tc.joinMachine, tc.joinConfiguration) + + // The apiserver endpoint should always match the incoming machine's network + expected := fmt.Sprintf("%v:%v", tc.joinMachine.GetScope().Network().APIServerELB.DNSName, kubeadm.APIServerBindPort) + if out.Discovery.BootstrapToken.APIServerEndpoint != expected { + t.Fatalf("join configuration apiserver endpoint: %q but expected %q", out.Discovery.BootstrapToken.APIServerEndpoint, expected) + } + + // The bootstrap token on the new join node configuration should be + // the same that is passed in + if out.Discovery.BootstrapToken.Token != tc.bootstrapToken { + t.Fatalf("bootstrap tokens did not match: got %q but expected %q", out.Discovery.BootstrapToken.Token, tc.bootstrapToken) + } + + // The passed in caCertHash should be appended to the existing hashes + if !contains(out.Discovery.BootstrapToken.CACertHashes, tc.caCertHash) { + t.Fatalf("did not find %q in %v", tc.caCertHash, out.Discovery.BootstrapToken.CACertHashes) + } + }) + } +} + +func TestSetJoinNodeConfigurationOverrides(t *testing.T) { + testcases := []struct { + name string + caCertHash string + bootstrapToken string + joinMachine *jm + joinConfiguration *kubeadmv1beta1.JoinConfiguration + }{ + { + name: "test node registration override values", + joinMachine: &jm{ + Scope: &actuators.Scope{ + ClusterStatus: &v1alpha1.AWSClusterProviderStatus{ + Network: v1alpha1.Network{ + APIServerELB: v1alpha1.ClassicELB{ + DNSName: "test.test", + }, + }, + }, + }, + Machine: &clusterv1.Machine{}, + }, + joinConfiguration: &kubeadmv1beta1.JoinConfiguration{ + NodeRegistration: kubeadmv1beta1.NodeRegistrationOptions{ + Name: "cat", + CRISocket: "unix:///var/run/docker/docker.sock", + KubeletExtraArgs: map[string]string{ + "cloud-provider": "gcp", + }, + }, + }, + }, + { + name: "should set the cloud-provider extra args even if args are nil", + joinMachine: &jm{ + Scope: &actuators.Scope{ + ClusterStatus: &v1alpha1.AWSClusterProviderStatus{}, + }, + Machine: &clusterv1.Machine{}, + }, + joinConfiguration: &kubeadmv1beta1.JoinConfiguration{}, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + out := kubeadm.SetJoinNodeConfigurationOverrides(tc.caCertHash, tc.bootstrapToken, tc.joinMachine, tc.joinConfiguration) + + if tc.joinConfiguration.NodeRegistration.Name != kubeadm.HostnameLookup && + tc.joinConfiguration.NodeRegistration.Name == out.NodeRegistration.Name { + t.Fatal("did not properly override the NodeRegistration.Name") + } + + if tc.joinConfiguration.NodeRegistration.CRISocket != kubeadm.ContainerdSocket && + tc.joinConfiguration.NodeRegistration.CRISocket == out.NodeRegistration.CRISocket { + t.Fatal("did not properly override the CRISocket") + } + + if out.NodeRegistration.KubeletExtraArgs["cloud-provider"] != kubeadm.CloudProvider { + t.Fatal("did not properly set the cloud-provider on the kubelet extra args") + } + }) + } +} + +func contains(haystack []string, needle string) bool { + for _, hay := range haystack { + if needle == hay { + return true + } + } + return false +}