From 98fb5c0e22b53a7032a85d3bfb7092eec9b6bdb2 Mon Sep 17 00:00:00 2001 From: Irfan Ur Rehman Date: Fri, 3 Feb 2017 23:38:20 +0530 Subject: [PATCH 1/3] [Federation] Add override flags options to kubefed init --- federation/pkg/kubefed/init/init.go | 128 ++++++++++++++++++++++------ hack/verify-flags/known-flags.txt | 2 + 2 files changed, 104 insertions(+), 26 deletions(-) diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index f0d0eec7913..93786a436b8 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -57,6 +57,7 @@ import ( "k8s.io/kubernetes/pkg/version" "github.com/spf13/cobra" + "sort" ) const ( @@ -116,6 +117,20 @@ var ( } hyperkubeImageName = "gcr.io/google_containers/hyperkube-amd64" + + apiserverDefaultArgs = map[string]string{ + "--bind-address": "0.0.0.0", + "--etcd-servers": "http://localhost:2379", + "--secure-port": "443", + "--client-ca-file": "/etc/federation/apiserver/ca.crt", + "--tls-cert-file": "/etc/federation/apiserver/server.crt", + "--tls-private-key-file": "/etc/federation/apiserver/server.key", + } + + cmDefaultArgs = map[string]string{ + "--kubeconfig": "/etc/federation/controller-manager/kubeconfig", + "--dns-provider-config": "", + } ) // NewCmdInit defines the `init` command that bootstraps a federation @@ -141,6 +156,8 @@ func NewCmdInit(cmdOut io.Writer, config util.AdminConfig) *cobra.Command { cmd.Flags().String("etcd-pv-capacity", "10Gi", "Size of persistent volume claim to be used for etcd.") cmd.Flags().Bool("etcd-persistent-storage", true, "Use persistent volume for etcd. Defaults to 'true'.") cmd.Flags().Bool("dry-run", false, "dry run without sending commands to server.") + cmd.Flags().String("apiserver-arg-overrides", "", "comma sapareted list of all or subset of the federation-apiserver arguments as \"--arg1=value1,--arg2=value2...\"") + cmd.Flags().String("controllermgr-arg-overrides", "", "comma sapareted list of all or subset of the federation-controller-manager arguments as \"--arg1=value1,--arg2=value2...\"") cmd.Flags().String("storage-backend", "etcd2", "The storage backend for persistence. Options: 'etcd2' (default), 'etcd3'.") cmd.Flags().String(apiserverServiceTypeFlag, string(v1.ServiceTypeLoadBalancer), "The type of service to create for federation API server. Options: 'LoadBalancer' (default), 'NodePort'.") cmd.Flags().String(apiserverAdvertiseAddressFlag, "", "Preferred address to advertise api server nodeport service. Valid only if '"+apiserverServiceTypeFlag+"=NodePort'.") @@ -168,6 +185,8 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman etcdPVCapacity := cmdutil.GetFlagString(cmd, "etcd-pv-capacity") etcdPersistence := cmdutil.GetFlagBool(cmd, "etcd-persistent-storage") dryRun := cmdutil.GetDryRunFlag(cmd) + apiserverArgOverrides := cmdutil.GetFlagString(cmd, "apiserver-arg-overrides") + cmArgOverrides := cmdutil.GetFlagString(cmd, "controllermgr-arg-overrides") storageBackend := cmdutil.GetFlagString(cmd, "storage-backend") apiserverServiceType := v1.ServiceType(cmdutil.GetFlagString(cmd, apiserverServiceTypeFlag)) apiserverAdvertiseAddress := cmdutil.GetFlagString(cmd, apiserverAdvertiseAddressFlag) @@ -245,7 +264,7 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman } // 6. Create federation API server - _, err = createAPIServer(hostClientset, initFlags.FederationSystemNamespace, serverName, image, serverCredName, advertiseAddress, storageBackend, pvc, dryRun) + _, err = createAPIServer(hostClientset, initFlags.FederationSystemNamespace, serverName, image, serverCredName, advertiseAddress, storageBackend, apiserverArgOverrides, pvc, dryRun) if err != nil { return err } @@ -266,7 +285,7 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman } // 7c. Create federation controller manager deployment. - _, err = createControllerManager(hostClientset, initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, cmName, image, cmKubeconfigName, dnsZoneName, dnsProvider, sa.Name, dryRun) + _, err = createControllerManager(hostClientset, initFlags.FederationSystemNamespace, initFlags.Name, svc.Name, cmName, image, cmKubeconfigName, dnsZoneName, dnsProvider, sa.Name, cmArgOverrides, dryRun) if err != nil { return err } @@ -518,24 +537,35 @@ func createPVC(clientset *client.Clientset, namespace, svcName, etcdPVCapacity s return clientset.Core().PersistentVolumeClaims(namespace).Create(pvc) } -func createAPIServer(clientset *client.Clientset, namespace, name, image, credentialsName, advertiseAddress, storageBackend string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) { +func createAPIServer(clientset *client.Clientset, namespace, name, image, credentialsName, advertiseAddress, storageBackend, apiserverArgOverrides string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) { command := []string{ "/hyperkube", "federation-apiserver", - "--bind-address=0.0.0.0", - "--etcd-servers=http://localhost:2379", - "--secure-port=443", - "--client-ca-file=/etc/federation/apiserver/ca.crt", - "--tls-cert-file=/etc/federation/apiserver/server.crt", - "--tls-private-key-file=/etc/federation/apiserver/server.key", - "--admission-control=NamespaceLifecycle", - fmt.Sprintf("--storage-backend=%s", storageBackend), + } + apiserverArgs := []string{} + apiserverArgsMap := apiserverDefaultArgs + + apiserverArgsMap["--storage-backend"] = storageBackend + if advertiseAddress != "" { + // This will be overridden with the override arg if user specifies one + apiserverArgsMap["--advertise-address"] = advertiseAddress } - if advertiseAddress != "" { - command = append(command, fmt.Sprintf("--advertise-address=%s", advertiseAddress)) + if apiserverArgOverrides != "" { + var err error + apiserverArgsMap, err = overrideArgs(apiserverArgsMap, apiserverArgOverrides) + if err != nil { + return nil, err + } } + for key, value := range apiserverArgsMap { + apiserverArgs = append(apiserverArgs, fmt.Sprintf("%s=%s", key, value)) + } + // This is needed for the unit test deep copy to get an exact match + sort.Strings(apiserverArgs) + command = append(command, apiserverArgs...) + dep := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -676,7 +706,34 @@ func createRoleBindings(clientset *client.Clientset, namespace, saName string, d return newRole, newRolebinding, err } -func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider, saName string, dryRun bool) (*extensions.Deployment, error) { +func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider, saName, cmArgOverrides string, dryRun bool) (*extensions.Deployment, error) { + command := []string{ + "/hyperkube", + "federation-controller-manager", + } + cmArgs := []string{} + cmArgsMap := cmDefaultArgs + + cmArgsMap["--master"] = fmt.Sprintf("https://%s", svcName) + cmArgsMap["--dns-provider"] = dnsProvider + cmArgsMap["--federation-name"] = name + cmArgsMap["--zone-name"] = dnsZoneName + + if cmArgOverrides != "" { + var err error + cmArgsMap, err = overrideArgs(cmArgsMap, cmArgOverrides) + if err != nil { + return nil, err + } + } + + for key, value := range cmArgsMap { + cmArgs = append(cmArgs, fmt.Sprintf("%s=%s", key, value)) + } + // This is needed for the unit test deep copy to get an exact match + sort.Strings(cmArgs) + command = append(command, cmArgs...) + dep := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: cmName, @@ -693,18 +750,9 @@ func createControllerManager(clientset *client.Clientset, namespace, name, svcNa Spec: api.PodSpec{ Containers: []api.Container{ { - Name: "controller-manager", - Image: image, - Command: []string{ - "/hyperkube", - "federation-controller-manager", - fmt.Sprintf("--master=https://%s", svcName), - "--kubeconfig=/etc/federation/controller-manager/kubeconfig", - fmt.Sprintf("--dns-provider=%s", dnsProvider), - "--dns-provider-config=", - fmt.Sprintf("--federation-name=%s", name), - fmt.Sprintf("--zone-name=%s", dnsZoneName), - }, + Name: "controller-manager", + Image: image, + Command: command, VolumeMounts: []api.VolumeMount{ { Name: kubeconfigName, @@ -746,6 +794,34 @@ func createControllerManager(clientset *client.Clientset, namespace, name, svcNa return clientset.Extensions().Deployments(namespace).Create(dep) } +func overrideArgs(serverArgs map[string]string, argOverrides string) (map[string]string, error) { + overrideArgs := strings.Split(argOverrides, ",") + argsMap := serverArgs + + for _, overrideArg := range overrideArgs { + overrideArgSplitted := strings.Split(overrideArg, "=") + if len(overrideArgSplitted) != 2 { + return nil, fmt.Errorf("Wrong format for override arg: %s", overrideArg) + } + found := false + for key := range argsMap { + if key == overrideArgSplitted[0] { + // Replace the default value by user specified value + argsMap[key] = overrideArgSplitted[1] + found = true + break + } + } + // This arg is not in defaults, append it. + // We do not care if the argument name or value used is not correct + // To verify the wrong args is the particular server apps job. + if found == false { + argsMap[overrideArgSplitted[0]] = overrideArgSplitted[1] + } + } + return argsMap, nil +} + func waitForPods(clientset *client.Clientset, fedPods []string, namespace string) error { err := wait.PollInfinite(podWaitInterval, func() (bool, error) { podCheck := len(fedPods) diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index f43cc1c6d53..3af526ff358 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -23,6 +23,7 @@ api-server-advertise-address api-server-service-type api-token api-version +apiserver-arg-overrides apiserver-count apiserver-count audit-log-maxage @@ -116,6 +117,7 @@ container-runtime container-runtime-endpoint contain-pod-resources contention-profiling +controllermgr-arg-overrides controller-start-interval cors-allowed-origins cpu-cfs-quota From 0ad1934d5a3b7df8ebfb052be3b96c2212473e01 Mon Sep 17 00:00:00 2001 From: Irfan Ur Rehman Date: Fri, 3 Feb 2017 23:43:23 +0530 Subject: [PATCH 2/3] [Federation] Unit test updates for override flags options in kubefed init --- federation/pkg/kubefed/init/init_test.go | 117 ++++++++++++++++------- 1 file changed, 81 insertions(+), 36 deletions(-) diff --git a/federation/pkg/kubefed/init/init_test.go b/federation/pkg/kubefed/init/init_test.go index 46dd1d20d20..7bbafda291b 100644 --- a/federation/pkg/kubefed/init/init_test.go +++ b/federation/pkg/kubefed/init/init_test.go @@ -27,6 +27,7 @@ import ( "net/http/httptest" "net/url" "strconv" + "sort" "strings" "testing" "time" @@ -90,6 +91,8 @@ func TestInitFederation(t *testing.T) { dnsProvider string storageBackend string dryRun string + apiserverArgOverrides string + cmArgOverrides string }{ { federation: "union", @@ -105,6 +108,8 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd2", dryRun: "", + apiserverArgOverrides: "--client-ca-file=override,--log-dir=override", + cmArgOverrides: "", }, { federation: "union", @@ -120,6 +125,8 @@ func TestInitFederation(t *testing.T) { dnsProvider: "", //test for default value of dns provider storageBackend: "etcd2", dryRun: "", + apiserverArgOverrides: "", + cmArgOverrides: "--dns-provider=override,--log-dir=override", }, { federation: "union", @@ -135,6 +142,8 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd2", dryRun: "valid-run", + apiserverArgOverrides: "--log-dir=override", + cmArgOverrides: "--log-dir=override", }, { federation: "union", @@ -150,6 +159,8 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd3", dryRun: "", + apiserverArgOverrides: "", + cmArgOverrides: "", }, { federation: "union", @@ -164,6 +175,8 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd3", dryRun: "", + apiserverArgOverrides: "", + cmArgOverrides: "", }, { federation: "union", @@ -179,6 +192,8 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd3", dryRun: "", + apiserverArgOverrides: "", + cmArgOverrides: "", }, } @@ -194,7 +209,7 @@ func TestInitFederation(t *testing.T) { } else { dnsProvider = "google-clouddns" //default value of dns-provider } - hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.image, dnsProvider, tc.etcdPersistence, tc.etcdPVCapacity, tc.storageBackend) + hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.image, dnsProvider, tc.etcdPersistence, tc.etcdPVCapacity, tc.storageBackend, tc.apiserverArgOverrides, tc.cmArgOverrides) if err != nil { t.Fatalf("[%d] unexpected error: %v", i, err) } @@ -210,6 +225,9 @@ func TestInitFederation(t *testing.T) { cmd.Flags().Set("host-cluster-context", "substrate") cmd.Flags().Set("dns-zone-name", tc.dnsZoneName) cmd.Flags().Set("image", tc.image) + cmd.Flags().Set("apiserver-arg-overrides", tc.apiserverArgOverrides) + cmd.Flags().Set("controllermgr-arg-overrides", tc.cmArgOverrides) + if tc.storageBackend != "" { cmd.Flags().Set("storage-backend", tc.storageBackend) } @@ -498,7 +516,7 @@ func TestCertsHTTPS(t *testing.T) { } } -func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, image, dnsProvider, etcdPersistence, etcdPVCapacity, storageProvider string) (cmdutil.Factory, error) { +func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, image, dnsProvider, etcdPersistence, etcdPVCapacity, storageProvider, apiserverOverrideArg, cmOverrideArg string) (cmdutil.Factory, error) { svcName := federationName + "-apiserver" svcUrlPrefix := "/api/v1/namespaces/federation-system/services" credSecretName := svcName + "-credentials" @@ -684,6 +702,39 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na nodeList := v1.NodeList{} nodeList.Items = append(nodeList.Items, node) + address := lbIp + if apiserverServiceType == v1.ServiceTypeNodePort { + if advertiseAddress != "" { + address = advertiseAddress + } else { + address = nodeIP + } + } + //apiserver.Spec.Template.Spec.Containers[0].Command = append(apiserver.Spec.Template.Spec.Containers[0].Command, fmt.Sprintf("--advertise-address=%s", address)) + + + apiserverCommand := []string{ + "/hyperkube", + "federation-apiserver", + } + apiserverArgs := []string{} + apiserverArgsMap := apiserverDefaultArgs + apiserverArgsMap["--storage-backend"] = storageProvider + //apiserverArgsMap["--advertise-address"] = ip + apiserverArgsMap["--advertise-address"] = address + if strings.Contains(apiserverOverrideArg, "--client-ca-file=override") { + apiserverArgsMap["--client-ca-file"] = "override" + } + if strings.Contains(apiserverOverrideArg, "--log-dir=override") { + apiserverArgsMap["--log-dir"] = "override" + } + + for key, value := range apiserverArgsMap { + apiserverArgs = append(apiserverArgs, fmt.Sprintf("%s=%s", key, value)) + } + sort.Strings(apiserverArgs) + apiserverCommand = append(apiserverCommand, apiserverArgs...) + apiserver := v1beta1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: "Deployment", @@ -705,20 +756,9 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na Spec: v1.PodSpec{ Containers: []v1.Container{ { - Name: "apiserver", - Image: image, - Command: []string{ - "/hyperkube", - "federation-apiserver", - "--bind-address=0.0.0.0", - "--etcd-servers=http://localhost:2379", - "--secure-port=443", - "--client-ca-file=/etc/federation/apiserver/ca.crt", - "--tls-cert-file=/etc/federation/apiserver/server.crt", - "--tls-private-key-file=/etc/federation/apiserver/server.key", - "--admission-control=NamespaceLifecycle", - fmt.Sprintf("--storage-backend=%s", storageProvider), - }, + Name: "apiserver", + Image: image, + Command: apiserverCommand, Ports: []v1.ContainerPort{ { Name: "https", @@ -784,15 +824,29 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na } } - address := lbIp - if apiserverServiceType == v1.ServiceTypeNodePort { - if advertiseAddress != "" { - address = advertiseAddress - } else { - address = nodeIP - } + cmCommand := []string{ + "/hyperkube", + "federation-controller-manager", } - apiserver.Spec.Template.Spec.Containers[0].Command = append(apiserver.Spec.Template.Spec.Containers[0].Command, fmt.Sprintf("--advertise-address=%s", address)) + cmArgs := []string{} + cmArgsMap := cmDefaultArgs + cmArgsMap["--federation-name"] = federationName + cmArgsMap["--zone-name"] = dnsZoneName + cmArgsMap["--master"] = "https://" + svcName + if strings.Contains(cmOverrideArg, "--dns-provider=override") { + cmArgsMap["--dns-provider"] = "override" + } else { + cmArgsMap["--dns-provider"] = dnsProvider + } + if strings.Contains(cmOverrideArg, "--log-dir=override") { + cmArgsMap["--log-dir"] = "override" + } + + for key, value := range cmArgsMap { + cmArgs = append(cmArgs, fmt.Sprintf("%s=%s", key, value)) + } + sort.Strings(cmArgs) + cmCommand = append(cmCommand, cmArgs...) cmName := federationName + "-controller-manager" cm := v1beta1.Deployment{ @@ -816,18 +870,9 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na Spec: v1.PodSpec{ Containers: []v1.Container{ { - Name: "controller-manager", - Image: image, - Command: []string{ - "/hyperkube", - "federation-controller-manager", - "--master=https://" + svcName, - "--kubeconfig=/etc/federation/controller-manager/kubeconfig", - fmt.Sprintf("--dns-provider=%s", dnsProvider), - "--dns-provider-config=", - fmt.Sprintf("--federation-name=%s", federationName), - fmt.Sprintf("--zone-name=%s", dnsZoneName), - }, + Name: "controller-manager", + Image: image, + Command: cmCommand, VolumeMounts: []v1.VolumeMount{ { Name: cmKubeconfigSecretName, From 9a56a75319b0c74d98b562a7ed4ad593422d9930 Mon Sep 17 00:00:00 2001 From: Irfan Ur Rehman Date: Sun, 5 Feb 2017 18:27:56 +0530 Subject: [PATCH 3/3] [Federation] Review comment fixes for add override flags options to kubefed init --- federation/pkg/kubefed/init/BUILD | 1 + federation/pkg/kubefed/init/init.go | 141 ++++++++--------- federation/pkg/kubefed/init/init_test.go | 185 ++++++++++++++--------- hack/verify-flags/known-flags.txt | 2 +- 4 files changed, 181 insertions(+), 148 deletions(-) diff --git a/federation/pkg/kubefed/init/BUILD b/federation/pkg/kubefed/init/BUILD index d216563f3d1..7c844d37147 100644 --- a/federation/pkg/kubefed/init/BUILD +++ b/federation/pkg/kubefed/init/BUILD @@ -54,6 +54,7 @@ go_test( "//vendor:k8s.io/apimachinery/pkg/api/resource", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/util/diff", + "//vendor:k8s.io/apimachinery/pkg/util/sets", "//vendor:k8s.io/client-go/dynamic", "//vendor:k8s.io/client-go/rest/fake", "//vendor:k8s.io/client-go/tools/clientcmd", diff --git a/federation/pkg/kubefed/init/init.go b/federation/pkg/kubefed/init/init.go index 93786a436b8..6307c752b86 100644 --- a/federation/pkg/kubefed/init/init.go +++ b/federation/pkg/kubefed/init/init.go @@ -117,20 +117,6 @@ var ( } hyperkubeImageName = "gcr.io/google_containers/hyperkube-amd64" - - apiserverDefaultArgs = map[string]string{ - "--bind-address": "0.0.0.0", - "--etcd-servers": "http://localhost:2379", - "--secure-port": "443", - "--client-ca-file": "/etc/federation/apiserver/ca.crt", - "--tls-cert-file": "/etc/federation/apiserver/server.crt", - "--tls-private-key-file": "/etc/federation/apiserver/server.key", - } - - cmDefaultArgs = map[string]string{ - "--kubeconfig": "/etc/federation/controller-manager/kubeconfig", - "--dns-provider-config": "", - } ) // NewCmdInit defines the `init` command that bootstraps a federation @@ -156,8 +142,8 @@ func NewCmdInit(cmdOut io.Writer, config util.AdminConfig) *cobra.Command { cmd.Flags().String("etcd-pv-capacity", "10Gi", "Size of persistent volume claim to be used for etcd.") cmd.Flags().Bool("etcd-persistent-storage", true, "Use persistent volume for etcd. Defaults to 'true'.") cmd.Flags().Bool("dry-run", false, "dry run without sending commands to server.") - cmd.Flags().String("apiserver-arg-overrides", "", "comma sapareted list of all or subset of the federation-apiserver arguments as \"--arg1=value1,--arg2=value2...\"") - cmd.Flags().String("controllermgr-arg-overrides", "", "comma sapareted list of all or subset of the federation-controller-manager arguments as \"--arg1=value1,--arg2=value2...\"") + cmd.Flags().String("apiserver-arg-overrides", "", "comma separated list of federation-apiserver arguments to override: Example \"--arg1=value1,--arg2=value2...\"") + cmd.Flags().String("controllermanager-arg-overrides", "", "comma separated list of federation-controller-manager arguments to override: Example \"--arg1=value1,--arg2=value2...\"") cmd.Flags().String("storage-backend", "etcd2", "The storage backend for persistence. Options: 'etcd2' (default), 'etcd3'.") cmd.Flags().String(apiserverServiceTypeFlag, string(v1.ServiceTypeLoadBalancer), "The type of service to create for federation API server. Options: 'LoadBalancer' (default), 'NodePort'.") cmd.Flags().String(apiserverAdvertiseAddressFlag, "", "Preferred address to advertise api server nodeport service. Valid only if '"+apiserverServiceTypeFlag+"=NodePort'.") @@ -185,8 +171,6 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman etcdPVCapacity := cmdutil.GetFlagString(cmd, "etcd-pv-capacity") etcdPersistence := cmdutil.GetFlagBool(cmd, "etcd-persistent-storage") dryRun := cmdutil.GetDryRunFlag(cmd) - apiserverArgOverrides := cmdutil.GetFlagString(cmd, "apiserver-arg-overrides") - cmArgOverrides := cmdutil.GetFlagString(cmd, "controllermgr-arg-overrides") storageBackend := cmdutil.GetFlagString(cmd, "storage-backend") apiserverServiceType := v1.ServiceType(cmdutil.GetFlagString(cmd, apiserverServiceTypeFlag)) apiserverAdvertiseAddress := cmdutil.GetFlagString(cmd, apiserverAdvertiseAddressFlag) @@ -203,6 +187,14 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman return fmt.Errorf("%s should be passed only with '%s=NodePort'", apiserverAdvertiseAddressFlag, apiserverServiceTypeFlag) } } + apiserverArgOverrides, err := marshallOverrides(cmdutil.GetFlagString(cmd, "apiserver-arg-overrides")) + if err != nil { + return fmt.Errorf("Error marshalling --apiserver-arg-overrides: %v", err) + } + cmArgOverrides, err := marshallOverrides(cmdutil.GetFlagString(cmd, "controllermanager-arg-overrides")) + if err != nil { + return fmt.Errorf("Error marshalling --controllermanager-arg-overrides: %v", err) + } hostFactory := config.HostFactory(initFlags.Host, initFlags.Kubeconfig) hostClientset, err := hostFactory.ClientSet() @@ -537,34 +529,28 @@ func createPVC(clientset *client.Clientset, namespace, svcName, etcdPVCapacity s return clientset.Core().PersistentVolumeClaims(namespace).Create(pvc) } -func createAPIServer(clientset *client.Clientset, namespace, name, image, credentialsName, advertiseAddress, storageBackend, apiserverArgOverrides string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) { +func createAPIServer(clientset *client.Clientset, namespace, name, image, credentialsName, advertiseAddress, storageBackend string, argOverrides map[string]string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) { command := []string{ "/hyperkube", "federation-apiserver", } - apiserverArgs := []string{} - apiserverArgsMap := apiserverDefaultArgs + argsMap := map[string]string{ + "--bind-address": "0.0.0.0", + "--etcd-servers": "http://localhost:2379", + "--secure-port": "443", + "--client-ca-file": "/etc/federation/apiserver/ca.crt", + "--tls-cert-file": "/etc/federation/apiserver/server.crt", + "--tls-private-key-file": "/etc/federation/apiserver/server.key", + "--admission-control": "NamespaceLifecycle", + } - apiserverArgsMap["--storage-backend"] = storageBackend + argsMap["--storage-backend"] = storageBackend if advertiseAddress != "" { - // This will be overridden with the override arg if user specifies one - apiserverArgsMap["--advertise-address"] = advertiseAddress + argsMap["--advertise-address"] = advertiseAddress } - if apiserverArgOverrides != "" { - var err error - apiserverArgsMap, err = overrideArgs(apiserverArgsMap, apiserverArgOverrides) - if err != nil { - return nil, err - } - } - - for key, value := range apiserverArgsMap { - apiserverArgs = append(apiserverArgs, fmt.Sprintf("%s=%s", key, value)) - } - // This is needed for the unit test deep copy to get an exact match - sort.Strings(apiserverArgs) - command = append(command, apiserverArgs...) + args := argMapsToArgStrings(argsMap, argOverrides) + command = append(command, args...) dep := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -706,33 +692,23 @@ func createRoleBindings(clientset *client.Clientset, namespace, saName string, d return newRole, newRolebinding, err } -func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider, saName, cmArgOverrides string, dryRun bool) (*extensions.Deployment, error) { +func createControllerManager(clientset *client.Clientset, namespace, name, svcName, cmName, image, kubeconfigName, dnsZoneName, dnsProvider, saName string, argOverrides map[string]string, dryRun bool) (*extensions.Deployment, error) { command := []string{ "/hyperkube", "federation-controller-manager", } - cmArgs := []string{} - cmArgsMap := cmDefaultArgs - - cmArgsMap["--master"] = fmt.Sprintf("https://%s", svcName) - cmArgsMap["--dns-provider"] = dnsProvider - cmArgsMap["--federation-name"] = name - cmArgsMap["--zone-name"] = dnsZoneName - - if cmArgOverrides != "" { - var err error - cmArgsMap, err = overrideArgs(cmArgsMap, cmArgOverrides) - if err != nil { - return nil, err - } + argsMap := map[string]string{ + "--kubeconfig": "/etc/federation/controller-manager/kubeconfig", + "--dns-provider-config": "", } - for key, value := range cmArgsMap { - cmArgs = append(cmArgs, fmt.Sprintf("%s=%s", key, value)) - } - // This is needed for the unit test deep copy to get an exact match - sort.Strings(cmArgs) - command = append(command, cmArgs...) + argsMap["--master"] = fmt.Sprintf("https://%s", svcName) + argsMap["--dns-provider"] = dnsProvider + argsMap["--federation-name"] = name + argsMap["--zone-name"] = dnsZoneName + + args := argMapsToArgStrings(argsMap, argOverrides) + command = append(command, args...) dep := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -794,34 +770,41 @@ func createControllerManager(clientset *client.Clientset, namespace, name, svcNa return clientset.Extensions().Deployments(namespace).Create(dep) } -func overrideArgs(serverArgs map[string]string, argOverrides string) (map[string]string, error) { - overrideArgs := strings.Split(argOverrides, ",") - argsMap := serverArgs +func marshallOverrides(overrideArgString string) (map[string]string, error) { + if overrideArgString == "" { + return nil, nil + } + argsMap := make(map[string]string) + overrideArgs := strings.Split(overrideArgString, ",") for _, overrideArg := range overrideArgs { - overrideArgSplitted := strings.Split(overrideArg, "=") - if len(overrideArgSplitted) != 2 { - return nil, fmt.Errorf("Wrong format for override arg: %s", overrideArg) + splitArg := strings.Split(overrideArg, "=") + if len(splitArg) != 2 { + return nil, fmt.Errorf("wrong format for override arg: %s", overrideArg) } - found := false - for key := range argsMap { - if key == overrideArgSplitted[0] { - // Replace the default value by user specified value - argsMap[key] = overrideArgSplitted[1] - found = true - break - } - } - // This arg is not in defaults, append it. - // We do not care if the argument name or value used is not correct - // To verify the wrong args is the particular server apps job. - if found == false { - argsMap[overrideArgSplitted[0]] = overrideArgSplitted[1] + key := strings.TrimSpace(splitArg[0]) + val := strings.TrimSpace(splitArg[1]) + if len(key) == 0 { + return nil, fmt.Errorf("wrong format for override arg: %s, arg name cannot be empty", overrideArg) } + argsMap[key] = val } return argsMap, nil } +func argMapsToArgStrings(argsMap, overrides map[string]string) []string { + for key, val := range overrides { + argsMap[key] = val + } + args := []string{} + for key, value := range argsMap { + args = append(args, fmt.Sprintf("%s=%s", key, value)) + } + // This is needed for the unit test deep copy to get an exact match + sort.Strings(args) + return args +} + func waitForPods(clientset *client.Clientset, fedPods []string, namespace string) error { err := wait.PollInfinite(podWaitInterval, func() (bool, error) { podCheck := len(fedPods) diff --git a/federation/pkg/kubefed/init/init_test.go b/federation/pkg/kubefed/init/init_test.go index 7bbafda291b..85e5eec273f 100644 --- a/federation/pkg/kubefed/init/init_test.go +++ b/federation/pkg/kubefed/init/init_test.go @@ -26,8 +26,8 @@ import ( "net/http" "net/http/httptest" "net/url" - "strconv" "sort" + "strconv" "strings" "testing" "time" @@ -37,6 +37,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest/fake" "k8s.io/client-go/tools/clientcmd" @@ -77,39 +78,39 @@ func TestInitFederation(t *testing.T) { defer kubefedtesting.RemoveFakeKubeconfigFiles(fakeKubeFiles) testCases := []struct { - federation string - kubeconfigGlobal string - kubeconfigExplicit string - dnsZoneName string - lbIP string - apiserverServiceType v1.ServiceType - advertiseAddress string - image string - etcdPVCapacity string - etcdPersistence string - expectedErr string - dnsProvider string - storageBackend string - dryRun string + federation string + kubeconfigGlobal string + kubeconfigExplicit string + dnsZoneName string + lbIP string + apiserverServiceType v1.ServiceType + advertiseAddress string + image string + etcdPVCapacity string + etcdPersistence string + expectedErr string + dnsProvider string + storageBackend string + dryRun string apiserverArgOverrides string cmArgOverrides string }{ { - federation: "union", - kubeconfigGlobal: fakeKubeFiles[0], - kubeconfigExplicit: "", - dnsZoneName: "example.test.", - lbIP: lbIP, - apiserverServiceType: v1.ServiceTypeLoadBalancer, - image: "example.test/foo:bar", - etcdPVCapacity: "5Gi", - etcdPersistence: "true", - expectedErr: "", - dnsProvider: "test-dns-provider", - storageBackend: "etcd2", - dryRun: "", + federation: "union", + kubeconfigGlobal: fakeKubeFiles[0], + kubeconfigExplicit: "", + dnsZoneName: "example.test.", + lbIP: lbIP, + apiserverServiceType: v1.ServiceTypeLoadBalancer, + image: "example.test/foo:bar", + etcdPVCapacity: "5Gi", + etcdPersistence: "true", + expectedErr: "", + dnsProvider: "test-dns-provider", + storageBackend: "etcd2", + dryRun: "", apiserverArgOverrides: "--client-ca-file=override,--log-dir=override", - cmArgOverrides: "", + cmArgOverrides: "--dns-provider=override,--log-dir=override", }, { federation: "union", @@ -125,8 +126,6 @@ func TestInitFederation(t *testing.T) { dnsProvider: "", //test for default value of dns provider storageBackend: "etcd2", dryRun: "", - apiserverArgOverrides: "", - cmArgOverrides: "--dns-provider=override,--log-dir=override", }, { federation: "union", @@ -142,8 +141,6 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd2", dryRun: "valid-run", - apiserverArgOverrides: "--log-dir=override", - cmArgOverrides: "--log-dir=override", }, { federation: "union", @@ -159,8 +156,6 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd3", dryRun: "", - apiserverArgOverrides: "", - cmArgOverrides: "", }, { federation: "union", @@ -175,8 +170,6 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd3", dryRun: "", - apiserverArgOverrides: "", - cmArgOverrides: "", }, { federation: "union", @@ -192,8 +185,6 @@ func TestInitFederation(t *testing.T) { dnsProvider: "test-dns-provider", storageBackend: "etcd3", dryRun: "", - apiserverArgOverrides: "", - cmArgOverrides: "", }, } @@ -226,7 +217,7 @@ func TestInitFederation(t *testing.T) { cmd.Flags().Set("dns-zone-name", tc.dnsZoneName) cmd.Flags().Set("image", tc.image) cmd.Flags().Set("apiserver-arg-overrides", tc.apiserverArgOverrides) - cmd.Flags().Set("controllermgr-arg-overrides", tc.cmArgOverrides) + cmd.Flags().Set("controllermanager-arg-overrides", tc.cmArgOverrides) if tc.storageBackend != "" { cmd.Flags().Set("storage-backend", tc.storageBackend) @@ -277,6 +268,64 @@ func TestInitFederation(t *testing.T) { } } +func TestMarshallAndMergeOverrides(t *testing.T) { + testCases := []struct { + overrideParams string + expectedSet sets.String + expectedErr string + }{ + { + overrideParams: "valid-format-param1=override1,valid-format-param2=override2", + expectedSet: sets.NewString("arg2=val2", "arg1=val1", "valid-format-param1=override1", "valid-format-param2=override2"), + expectedErr: "", + }, + { + overrideParams: "valid-format-param1=override1,arg1=override1", + expectedSet: sets.NewString("arg2=val2", "arg1=override1", "valid-format-param1=override1"), + expectedErr: "", + }, + { + overrideParams: "zero-value-arg=", + expectedSet: sets.NewString("arg2=val2", "arg1=val1", "zero-value-arg="), + expectedErr: "", + }, + { + overrideParams: "wrong-format-arg", + expectedErr: "wrong format for override arg: wrong-format-arg", + }, + { + overrideParams: "wrong-format-arg=override=wrong-format-arg=override", + expectedErr: "wrong format for override arg: wrong-format-arg=override=wrong-format-arg=override", + }, + { + overrideParams: "=wrong-format-only-value", + expectedErr: "wrong format for override arg: =wrong-format-only-value, arg name cannot be empty", + }, + } + + for i, tc := range testCases { + args, err := marshallOverrides(tc.overrideParams) + if tc.expectedErr == "" { + origArgs := map[string]string{ + "arg1": "val1", + "arg2": "val2", + } + merged := argMapsToArgStrings(origArgs, args) + + got := sets.NewString(merged...) + want := tc.expectedSet + + if !got.Equal(want) { + t.Errorf("[%d] unexpected output: got: %v, want: %v", i, got, want) + } + } else { + if err.Error() != tc.expectedErr { + t.Errorf("[%d] unexpected error output: got: %s, want: %s", i, err.Error(), tc.expectedErr) + } + } + } +} + // TestCertsTLS tests TLS handshake with client authentication for any server // name. There is a separate test below to test the certificate generation // end-to-end over HTTPS. @@ -710,27 +759,28 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na address = nodeIP } } - //apiserver.Spec.Template.Spec.Containers[0].Command = append(apiserver.Spec.Template.Spec.Containers[0].Command, fmt.Sprintf("--advertise-address=%s", address)) - apiserverCommand := []string{ "/hyperkube", "federation-apiserver", } - apiserverArgs := []string{} - apiserverArgsMap := apiserverDefaultArgs - apiserverArgsMap["--storage-backend"] = storageProvider - //apiserverArgsMap["--advertise-address"] = ip - apiserverArgsMap["--advertise-address"] = address - if strings.Contains(apiserverOverrideArg, "--client-ca-file=override") { - apiserverArgsMap["--client-ca-file"] = "override" - } - if strings.Contains(apiserverOverrideArg, "--log-dir=override") { - apiserverArgsMap["--log-dir"] = "override" + apiserverArgs := []string{ + "--bind-address=0.0.0.0", + "--etcd-servers=http://localhost:2379", + "--secure-port=443", + "--tls-cert-file=/etc/federation/apiserver/server.crt", + "--tls-private-key-file=/etc/federation/apiserver/server.key", + "--admission-control=NamespaceLifecycle", + fmt.Sprintf("--storage-backend=%s", storageProvider), + fmt.Sprintf("--advertise-address=%s", address), } - for key, value := range apiserverArgsMap { - apiserverArgs = append(apiserverArgs, fmt.Sprintf("%s=%s", key, value)) + if apiserverOverrideArg != "" { + apiserverArgs = append(apiserverArgs, "--client-ca-file=override") + apiserverArgs = append(apiserverArgs, "--log-dir=override") + + } else { + apiserverArgs = append(apiserverArgs, "--client-ca-file=/etc/federation/apiserver/ca.crt") } sort.Strings(apiserverArgs) apiserverCommand = append(apiserverCommand, apiserverArgs...) @@ -828,23 +878,22 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na "/hyperkube", "federation-controller-manager", } - cmArgs := []string{} - cmArgsMap := cmDefaultArgs - cmArgsMap["--federation-name"] = federationName - cmArgsMap["--zone-name"] = dnsZoneName - cmArgsMap["--master"] = "https://" + svcName - if strings.Contains(cmOverrideArg, "--dns-provider=override") { - cmArgsMap["--dns-provider"] = "override" - } else { - cmArgsMap["--dns-provider"] = dnsProvider - } - if strings.Contains(cmOverrideArg, "--log-dir=override") { - cmArgsMap["--log-dir"] = "override" + + cmArgs := []string{ + "--kubeconfig=/etc/federation/controller-manager/kubeconfig", + "--dns-provider-config=", + fmt.Sprintf("--federation-name=%s", federationName), + fmt.Sprintf("--zone-name=%s", dnsZoneName), + fmt.Sprintf("--master=https://%s", svcName), } - for key, value := range cmArgsMap { - cmArgs = append(cmArgs, fmt.Sprintf("%s=%s", key, value)) + if cmOverrideArg != "" { + cmArgs = append(cmArgs, "--dns-provider=override") + cmArgs = append(cmArgs, "--log-dir=override") + } else { + cmArgs = append(cmArgs, fmt.Sprintf("--dns-provider=%s", dnsProvider)) } + sort.Strings(cmArgs) cmCommand = append(cmCommand, cmArgs...) diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 3af526ff358..0c55cfa71b1 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -117,7 +117,7 @@ container-runtime container-runtime-endpoint contain-pod-resources contention-profiling -controllermgr-arg-overrides +controllermanager-arg-overrides controller-start-interval cors-allowed-origins cpu-cfs-quota