Merge pull request #46247 from marun/fed-override-etcd-default-image

Automatic merge from submit-queue (batch tested with PRs 46201, 45952, 45427, 46247, 46062)

[Federation][kubefed]: Add support for etcd image override

This PR adds support for overriding the default etcd image used by ``kubefed init`` by providing an argument to ``--etcd-image``.  This is primarily intended to allow consumers like openshift to provide a different default, but as a nice side-effect supports code-free validation of non-default etcd images. 

**Release note**:

```release-note
'kubefed init' now supports overriding the default etcd image name with the --etcd-image parameter.
```
cc: @kubernetes/sig-federation-pr-reviews

federation/cmd/kubefed/app/kubefed.go

@@ -28,13 +28,16 @@ import (
 	_ "k8s.io/kubernetes/pkg/version/prometheus" // for version metric registration
 )
 
-const hyperkubeImageName = "gcr.io/google_containers/hyperkube-amd64"
+const (
+	hyperkubeImageName = "gcr.io/google_containers/hyperkube-amd64"
+	defaultEtcdImage   = "gcr.io/google_containers/etcd:3.0.17"
+)
 
 func Run() error {
 	logs.InitLogs()
 	defer logs.FlushLogs()
 
-	defaultImage := fmt.Sprintf("%s:%s", hyperkubeImageName, version.Get())
-	cmd := kubefed.NewKubeFedCommand(cmdutil.NewFactory(nil), os.Stdin, os.Stdout, os.Stderr, defaultImage)
+	defaultServerImage := fmt.Sprintf("%s:%s", hyperkubeImageName, version.Get())
+	cmd := kubefed.NewKubeFedCommand(cmdutil.NewFactory(nil), os.Stdin, os.Stdout, os.Stderr, defaultServerImage, defaultEtcdImage)
 	return cmd.Execute()
 }

federation/pkg/kubefed/init/init.go

@@ -134,9 +134,10 @@ type initFederation struct {
 
 type initFederationOptions struct {
 	dnsZoneName                      string
-	image                            string
+	serverImage                      string
 	dnsProvider                      string
 	dnsProviderConfig                string
+	etcdImage                        string
 	etcdPVCapacity                   string
 	etcdPersistentStorage            bool
 	dryRun                           bool
@@ -151,11 +152,12 @@ type initFederationOptions struct {
 	apiServerEnableTokenAuth         bool
 }
 
-func (o *initFederationOptions) Bind(flags *pflag.FlagSet, defaultImage string) {
+func (o *initFederationOptions) Bind(flags *pflag.FlagSet, defaultServerImage, defaultEtcdImage string) {
 	flags.StringVar(&o.dnsZoneName, "dns-zone-name", "", "DNS suffix for this federation. Federated Service DNS names are published with this suffix.")
-	flags.StringVar(&o.image, "image", defaultImage, "Image to use for federation API server and controller manager binaries.")
+	flags.StringVar(&o.serverImage, "image", defaultServerImage, "Image to use for federation API server and controller manager binaries.")
 	flags.StringVar(&o.dnsProvider, "dns-provider", "", "Dns provider to be used for this deployment.")
 	flags.StringVar(&o.dnsProviderConfig, "dns-provider-config", "", "Config file path on local file system for configuring DNS provider.")
+	flags.StringVar(&o.etcdImage, "etcd-image", defaultEtcdImage, "Image to use for etcd server.")
 	flags.StringVar(&o.etcdPVCapacity, "etcd-pv-capacity", "10Gi", "Size of persistent volume claim to be used for etcd.")
 	flags.BoolVar(&o.etcdPersistentStorage, "etcd-persistent-storage", true, "Use persistent volume for etcd. Defaults to 'true'.")
 	flags.BoolVar(&o.dryRun, "dry-run", false, "dry run without sending commands to server.")
@@ -169,7 +171,7 @@ func (o *initFederationOptions) Bind(flags *pflag.FlagSet, defaultImage string)
 
 // NewCmdInit defines the `init` command that bootstraps a federation
 // control plane inside a set of host clusters.
-func NewCmdInit(cmdOut io.Writer, config util.AdminConfig, defaultImage string) *cobra.Command {
+func NewCmdInit(cmdOut io.Writer, config util.AdminConfig, defaultServerImage, defaultEtcdImage string) *cobra.Command {
 	opts := &initFederation{}
 
 	cmd := &cobra.Command{
@@ -185,7 +187,7 @@ func NewCmdInit(cmdOut io.Writer, config util.AdminConfig, defaultImage string)
 
 	flags := cmd.Flags()
 	opts.commonOptions.Bind(flags)
-	opts.options.Bind(flags, defaultImage)
+	opts.options.Bind(flags, defaultServerImage, defaultEtcdImage)
 
 	return cmd
 }
@@ -341,7 +343,7 @@ func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
 
 	fmt.Fprint(cmdOut, "Creating federation component deployments...")
 	glog.V(4).Info("Creating federation control plane components")
-	_, err = createAPIServer(hostClientset, i.commonOptions.FederationSystemNamespace, serverName, i.commonOptions.Name, i.options.image, advertiseAddress, serverCredName, i.options.apiServerEnableHTTPBasicAuth, i.options.apiServerEnableTokenAuth, i.options.apiServerOverrides, pvc, i.options.dryRun)
+	_, err = createAPIServer(hostClientset, i.commonOptions.FederationSystemNamespace, serverName, i.commonOptions.Name, i.options.serverImage, i.options.etcdImage, advertiseAddress, serverCredName, i.options.apiServerEnableHTTPBasicAuth, i.options.apiServerEnableTokenAuth, i.options.apiServerOverrides, pvc, i.options.dryRun)
 	if err != nil {
 		return err
 	}
@@ -376,7 +378,7 @@ func (i *initFederation) Run(cmdOut io.Writer, config util.AdminConfig) error {
 
 	glog.V(4).Info("Creating federation controller manager deployment")
 
-	_, err = createControllerManager(hostClientset, i.commonOptions.FederationSystemNamespace, i.commonOptions.Name, svc.Name, cmName, i.options.image, cmKubeconfigName, i.options.dnsZoneName, i.options.dnsProvider, i.options.dnsProviderConfig, sa.Name, dnsProviderSecret, i.options.controllerManagerOverrides, i.options.dryRun)
+	_, err = createControllerManager(hostClientset, i.commonOptions.FederationSystemNamespace, i.commonOptions.Name, svc.Name, cmName, i.options.serverImage, cmKubeconfigName, i.options.dnsZoneName, i.options.dnsProvider, i.options.dnsProviderConfig, sa.Name, dnsProviderSecret, i.options.controllerManagerOverrides, i.options.dryRun)
 	if err != nil {
 		return err
 	}
@@ -670,7 +672,7 @@ func createPVC(clientset client.Interface, namespace, svcName, federationName, e
 	return clientset.Core().PersistentVolumeClaims(namespace).Create(pvc)
 }
 
-func createAPIServer(clientset client.Interface, namespace, name, federationName, image, advertiseAddress, credentialsName string, hasHTTPBasicAuthFile, hasTokenAuthFile bool, argOverrides map[string]string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) {
+func createAPIServer(clientset client.Interface, namespace, name, federationName, serverImage, etcdImage, advertiseAddress, credentialsName string, hasHTTPBasicAuthFile, hasTokenAuthFile bool, argOverrides map[string]string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) {
 	command := []string{
 		"/hyperkube",
 		"federation-apiserver",
@@ -717,7 +719,7 @@ func createAPIServer(clientset client.Interface, namespace, name, federationName
 					Containers: []api.Container{
 						{
 							Name:    "apiserver",
-							Image:   image,
+							Image:   serverImage,
 							Command: command,
 							Ports: []api.ContainerPort{
 								{
@@ -739,7 +741,7 @@ func createAPIServer(clientset client.Interface, namespace, name, federationName
 						},
 						{
 							Name:  "etcd",
-							Image: "gcr.io/google_containers/etcd:3.0.17",
+							Image: etcdImage,
 							Command: []string{
 								"/usr/local/bin/etcd",
 								"--data-dir",

federation/pkg/kubefed/init/init_test.go

@@ -95,7 +95,8 @@ func TestInitFederation(t *testing.T) {
 		lbIP                         string
 		apiserverServiceType         v1.ServiceType
 		advertiseAddress             string
-		image                        string
+		serverImage                  string
+		etcdImage                    string
 		etcdPVCapacity               string
 		etcdPersistence              string
 		expectedErr                  string
@@ -115,7 +116,7 @@ func TestInitFederation(t *testing.T) {
 			dnsZoneName:           "example.test.",
 			lbIP:                  lbIP,
 			apiserverServiceType:  v1.ServiceTypeLoadBalancer,
-			image:                 "example.test/foo:bar",
+			serverImage:           "example.test/foo:bar",
 			etcdPVCapacity:        "5Gi",
 			etcdPersistence:       "true",
 			expectedErr:           "",
@@ -132,7 +133,7 @@ func TestInitFederation(t *testing.T) {
 			dnsZoneName:          "example.test.",
 			lbIP:                 lbIP,
 			apiserverServiceType: v1.ServiceTypeLoadBalancer,
-			image:                "example.test/foo:bar",
+			serverImage:          "example.test/foo:bar",
 			etcdPVCapacity:       "", //test for default value of pvc-size
 			etcdPersistence:      "true",
 			expectedErr:          "",
@@ -145,7 +146,7 @@ func TestInitFederation(t *testing.T) {
 			dnsZoneName:          "example.test.",
 			lbIP:                 lbIP,
 			apiserverServiceType: v1.ServiceTypeLoadBalancer,
-			image:                "example.test/foo:bar",
+			serverImage:          "example.test/foo:bar",
 			etcdPVCapacity:       "",
 			etcdPersistence:      "true",
 			expectedErr:          "",
@@ -158,7 +159,7 @@ func TestInitFederation(t *testing.T) {
 			dnsZoneName:          "example.test.",
 			lbIP:                 lbIP,
 			apiserverServiceType: v1.ServiceTypeLoadBalancer,
-			image:                "example.test/foo:bar",
+			serverImage:          "example.test/foo:bar",
 			etcdPVCapacity:       "5Gi",
 			etcdPersistence:      "false",
 			expectedErr:          "",
@@ -170,7 +171,7 @@ func TestInitFederation(t *testing.T) {
 			kubeconfigExplicit:   "",
 			dnsZoneName:          "example.test.",
 			apiserverServiceType: v1.ServiceTypeNodePort,
-			image:                "example.test/foo:bar",
+			serverImage:          "example.test/foo:bar",
 			etcdPVCapacity:       "5Gi",
 			etcdPersistence:      "true",
 			expectedErr:          "",
@@ -183,7 +184,7 @@ func TestInitFederation(t *testing.T) {
 			dnsZoneName:          "example.test.",
 			apiserverServiceType: v1.ServiceTypeNodePort,
 			advertiseAddress:     nodeIP,
-			image:                "example.test/foo:bar",
+			serverImage:          "example.test/foo:bar",
 			etcdPVCapacity:       "5Gi",
 			etcdPersistence:      "true",
 			expectedErr:          "",
@@ -196,7 +197,8 @@ func TestInitFederation(t *testing.T) {
 			dnsZoneName:          "example.test.",
 			apiserverServiceType: v1.ServiceTypeNodePort,
 			advertiseAddress:     nodeIP,
-			image:                "example.test/foo:bar",
+			serverImage:          "example.test/foo:bar",
+			etcdImage:            "gcr.io/google_containers/etcd:latest",
 			etcdPVCapacity:       "5Gi",
 			etcdPersistence:      "true",
 			expectedErr:          "",
@@ -207,6 +209,8 @@ func TestInitFederation(t *testing.T) {
 		},
 	}
 
+	defaultEtcdImage := "gcr.io/google_containers/etcd:3.0.17"
+
 	//TODO: implement a negative case for dry run
 
 	for i, tc := range testCases {
@@ -234,7 +238,13 @@ func TestInitFederation(t *testing.T) {
 		}
 		defer os.Remove(tmpDirPath)
 
-		hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.image, tc.dnsProvider, tc.dnsProviderConfig, tc.etcdPersistence, tc.etcdPVCapacity, tc.apiserverArgOverrides, tc.cmArgOverrides, tmpDirPath, tc.apiserverEnableHTTPBasicAuth, tc.apiserverEnableTokenAuth, tc.isRBACAPIAvailable)
+		// If tc.etcdImage is set, setting the etcd image via the flag will be
+		// validated.  If not set, the default value will be validated.
+		if tc.etcdImage == "" {
+			tc.etcdImage = defaultEtcdImage
+		}
+
+		hostFactory, err := fakeInitHostFactory(tc.apiserverServiceType, tc.federation, util.DefaultFederationSystemNamespace, tc.advertiseAddress, tc.lbIP, tc.dnsZoneName, tc.serverImage, tc.etcdImage, tc.dnsProvider, tc.dnsProviderConfig, tc.etcdPersistence, tc.etcdPVCapacity, tc.apiserverArgOverrides, tc.cmArgOverrides, tmpDirPath, tc.apiserverEnableHTTPBasicAuth, tc.apiserverEnableTokenAuth, tc.isRBACAPIAvailable)
 		if err != nil {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
@@ -244,12 +254,13 @@ func TestInitFederation(t *testing.T) {
 			t.Fatalf("[%d] unexpected error: %v", i, err)
 		}
 
-		cmd := NewCmdInit(buf, adminConfig, "image")
+		cmd := NewCmdInit(buf, adminConfig, "serverImage", defaultEtcdImage)
 
 		cmd.Flags().Set("kubeconfig", tc.kubeconfigExplicit)
 		cmd.Flags().Set("host-cluster-context", "substrate")
 		cmd.Flags().Set("dns-zone-name", tc.dnsZoneName)
-		cmd.Flags().Set("image", tc.image)
+		cmd.Flags().Set("image", tc.serverImage)
+		cmd.Flags().Set("etcd-image", tc.etcdImage)
 		cmd.Flags().Set("dns-provider", tc.dnsProvider)
 		cmd.Flags().Set("apiserver-arg-overrides", tc.apiserverArgOverrides)
 		cmd.Flags().Set("controllermanager-arg-overrides", tc.cmArgOverrides)
@@ -605,7 +616,7 @@ func TestCertsHTTPS(t *testing.T) {
 	}
 }
 
-func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, image, dnsProvider, dnsProviderConfig, etcdPersistence, etcdPVCapacity, apiserverOverrideArg, cmOverrideArg, tmpDirPath string, apiserverEnableHTTPBasicAuth, apiserverEnableTokenAuth, isRBACAPIAvailable bool) (cmdutil.Factory, error) {
+func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, namespaceName, advertiseAddress, lbIp, dnsZoneName, serverImage, etcdImage, dnsProvider, dnsProviderConfig, etcdPersistence, etcdPVCapacity, apiserverOverrideArg, cmOverrideArg, tmpDirPath string, apiserverEnableHTTPBasicAuth, apiserverEnableTokenAuth, isRBACAPIAvailable bool) (cmdutil.Factory, error) {
 	svcName := federationName + "-apiserver"
 	svcUrlPrefix := "/api/v1/namespaces/federation-system/services"
 	credSecretName := svcName + "-credentials"
@@ -892,7 +903,7 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 					Containers: []v1.Container{
 						{
 							Name:    "apiserver",
-							Image:   image,
+							Image:   serverImage,
 							Command: apiserverCommand,
 							Ports: []v1.ContainerPort{
 								{
@@ -914,7 +925,7 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 						},
 						{
 							Name:  "etcd",
-							Image: "gcr.io/google_containers/etcd:3.0.17",
+							Image: etcdImage,
 							Command: []string{
 								"/usr/local/bin/etcd",
 								"--data-dir",
@@ -1009,7 +1020,7 @@ func fakeInitHostFactory(apiserverServiceType v1.ServiceType, federationName, na
 					Containers: []v1.Container{
 						{
 							Name:    "controller-manager",
-							Image:   image,
+							Image:   serverImage,
 							Command: cmCommand,
 							VolumeMounts: []v1.VolumeMount{
 								{

federation/pkg/kubefed/kubefed.go

@@ -31,7 +31,7 @@ import (
 )
 
 // NewKubeFedCommand creates the `kubefed` command and its nested children.
-func NewKubeFedCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer, defaultImage string) *cobra.Command {
+func NewKubeFedCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer, defaultServerImage, defaultEtcdImage string) *cobra.Command {
 	// Parent command to which all subcommands are added.
 	cmds := &cobra.Command{
 		Use:   "kubefed",
@@ -53,7 +53,7 @@ func NewKubeFedCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer, defa
 		{
 			Message: "Basic Commands:",
 			Commands: []*cobra.Command{
-				kubefedinit.NewCmdInit(out, util.NewAdminConfig(clientcmd.NewDefaultPathOptions()), defaultImage),
+				kubefedinit.NewCmdInit(out, util.NewAdminConfig(clientcmd.NewDefaultPathOptions()), defaultServerImage, defaultEtcdImage),
 				NewCmdJoin(f, out, util.NewAdminConfig(clientcmd.NewDefaultPathOptions())),
 				NewCmdUnjoin(f, out, err, util.NewAdminConfig(clientcmd.NewDefaultPathOptions())),
 			},

hack/verify-flags/known-flags.txt

@@ -207,6 +207,7 @@ etcd-cafile
 etcd-certfile
 etcd-config
 etcd-keyfile
+etcd-image
 etcd-metrics-scrape-uri
 etcd-metrics-scrape-uri
 etcd-mutation-timeout