Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm: add kubeadm phase addons command #51171

Merged
merged 1 commit into from
Sep 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ func (i *Init) Run(out io.Writer) error {
return err
}

if err := dnsaddonphase.EnsureDNSAddon(i.cfg, client, k8sVersion); err != nil {
if err := dnsaddonphase.EnsureDNSAddon(i.cfg, client); err != nil {
return err
}

Expand Down
4 changes: 4 additions & 0 deletions cmd/kubeadm/app/cmd/phases/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ load(
go_library(
name = "go_default_library",
srcs = [
"addons.go",
"bootstraptoken.go",
"certs.go",
"controlplane.go",
Expand All @@ -28,6 +29,8 @@ go_library(
"//cmd/kubeadm/app/cmd/util:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/features:go_default_library",
"//cmd/kubeadm/app/phases/addons/dns:go_default_library",
"//cmd/kubeadm/app/phases/addons/proxy:go_default_library",
"//cmd/kubeadm/app/phases/bootstraptoken/clusterinfo:go_default_library",
"//cmd/kubeadm/app/phases/bootstraptoken/node:go_default_library",
"//cmd/kubeadm/app/phases/certs:go_default_library",
Expand All @@ -52,6 +55,7 @@ go_library(
go_test(
name = "go_default_test",
srcs = [
"addons_test.go",
"certs_test.go",
"controlplane_test.go",
"etcd_test.go",
Expand Down
151 changes: 151 additions & 0 deletions cmd/kubeadm/app/cmd/phases/addons.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
Copyright 2017 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 phases

import (
"github.com/spf13/cobra"

clientset "k8s.io/client-go/kubernetes"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
dnsaddon "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns"
proxyaddon "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
"k8s.io/kubernetes/pkg/api"
)

// NewCmdAddon returns the addon Cobra command
func NewCmdAddon() *cobra.Command {
cmd := &cobra.Command{
Use: "addon <addon-name>",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luxas having <addon-name>here is different that other phases; IMO opinion it provides a slightly better user experience, but however I would prefer to have it consistent across phases.
I leave the final decision to you (eventually creating another issue for changing other phases as well)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this, we can make the other phases consistent with this later (feel free to do PRs 😉)

Aliases: []string{"addons"},
Short: "Install an addon to a Kubernetes cluster.",
RunE: cmdutil.SubCmdRunE("addon"),
}

cmd.AddCommand(getAddonsSubCommands()...)
return cmd
}

// EnsureAllAddons install all addons to a Kubernetes cluster.
func EnsureAllAddons(cfg *kubeadmapi.MasterConfiguration, client clientset.Interface) error {

addonActions := []func(cfg *kubeadmapi.MasterConfiguration, client clientset.Interface) error{
dnsaddon.EnsureDNSAddon,
proxyaddon.EnsureProxyAddon,
}

for _, action := range addonActions {
err := action(cfg, client)
if err != nil {
return err
}
}

return nil
}

// getAddonsSubCommands returns sub commands for addons phase
func getAddonsSubCommands() []*cobra.Command {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit test like some other phases have? non-blocking but would be a plus

cfg := &kubeadmapiext.MasterConfiguration{}
// Default values for the cobra help text
api.Scheme.Default(cfg)

var cfgPath, kubeConfigFile string
var subCmds []*cobra.Command

subCmdProperties := []struct {
use string
short string
cmdFunc func(cfg *kubeadmapi.MasterConfiguration, client clientset.Interface) error
}{
{
use: "all",
short: "Install all addons to a Kubernetes cluster",
cmdFunc: EnsureAllAddons,
},
{
use: "kube-dns",
short: "Install the kube-dns addon to a Kubernetes cluster.",
cmdFunc: dnsaddon.EnsureDNSAddon,
},
{
use: "kube-proxy",
short: "Install the kube-proxy addon to a Kubernetes cluster.",
cmdFunc: proxyaddon.EnsureProxyAddon,
},
}

for _, properties := range subCmdProperties {
// Creates the UX Command
cmd := &cobra.Command{
Use: properties.use,
Short: properties.short,
Run: runAddonsCmdFunc(properties.cmdFunc, cfg, cfgPath),
}

// Add flags to the command
cmd.Flags().StringVar(&kubeConfigFile, "kubeconfig", "/etc/kubernetes/admin.conf", "The KubeConfig file to use for talking to the cluster")
cmd.Flags().StringVar(&cfgPath, "config", cfgPath, "Path to kubeadm config file (WARNING: Usage of a configuration file is experimental)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add flags for these fields:
common:

  • KubernetesVersion
  • ImageRepository

proxy:

  • AdvertiseAddress
  • BindPort
  • PodSubnet

dns:

  • DNSDomain

cmd.Flags().StringVar(&cfg.KubernetesVersion, "kubernetes-version", cfg.KubernetesVersion, `Choose a specific Kubernetes version for the control plane.`)
cmd.Flags().StringVar(&cfg.ImageRepository, "image-repository", cfg.ImageRepository, `Choose a container registry to pull control plane images from.`)

if properties.use == "all" || properties.use == "kube-proxy" {
cmd.Flags().StringVar(&cfg.API.AdvertiseAddress, "apiserver-advertise-address", cfg.API.AdvertiseAddress, `The IP address the API Server will advertise it's listening on. 0.0.0.0 means the default network interface's address.`)
cmd.Flags().Int32Var(&cfg.API.BindPort, "apiserver-bind-port", cfg.API.BindPort, `Port for the API Server to bind to.`)
cmd.Flags().StringVar(&cfg.Networking.PodSubnet, "pod-network-cidr", cfg.Networking.PodSubnet, `Specify range of IP addresses for the pod network; if set, the control plane will automatically allocate CIDRs for every node.`)
}

if properties.use == "all" || properties.use == "kube-dns" {
cmd.Flags().StringVar(&cfg.Networking.DNSDomain, "service-dns-domain", cfg.Networking.DNSDomain, `Use alternative domain for services, e.g. "myorg.internal.`)
}
subCmds = append(subCmds, cmd)
}

return subCmds
}

// runAddonsCmdFunc creates a cobra.Command Run function, by composing the call to the given cmdFunc with necessary additional steps (e.g preparation of input parameters)
func runAddonsCmdFunc(cmdFunc func(cfg *kubeadmapi.MasterConfiguration, client clientset.Interface) error, cfg *kubeadmapiext.MasterConfiguration, cfgPath string) func(cmd *cobra.Command, args []string) {

// the following statement build a clousure that wraps a call to a cmdFunc, binding
// the function itself with the specific parameters of each sub command.
// Please note that specific parameter should be passed as value, while other parameters - passed as reference -
// are shared between sub commands and gets access to current value e.g. flags value.

return func(cmd *cobra.Command, args []string) {
if err := validation.ValidateMixedArguments(cmd.Flags()); err != nil {
kubeadmutil.CheckErr(err)
}

var kubeConfigFile string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not able to actually get any of these commands working. Because of this closure, the kubeconfig value from the flag won't ever get used.

$ _output/bin/kubeadm alpha phase addon all --kubeconfig test
failed to load admin kubeconfig [open : no such file or directory]

cc @luxas @andrewrynhard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can have a fix soon. Thanks @r2d4

internalcfg := &kubeadmapi.MasterConfiguration{}
api.Scheme.Convert(cfg, internalcfg, nil)
client, err := kubeconfigutil.ClientSetFromFile(kubeConfigFile)
kubeadmutil.CheckErr(err)
internalcfg, err = configutil.ConfigFileAndDefaultsToInternalConfig(cfgPath, cfg)
kubeadmutil.CheckErr(err)

// Execute the cmdFunc
err = cmdFunc(internalcfg, client)
kubeadmutil.CheckErr(err)
}
}
72 changes: 72 additions & 0 deletions cmd/kubeadm/app/cmd/phases/addons_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
Copyright 2017 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 phases

import (
"testing"

// required for triggering api machinery startup when running unit tests
_ "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/install"

cmdtestutil "k8s.io/kubernetes/cmd/kubeadm/test/cmd"
)

func TestAddonsSubCommandsHasFlags(t *testing.T) {

subCmds := getAddonsSubCommands()

commonFlags := []string{
"kubeconfig",
"config",
"kubernetes-version",
"image-repository",
}

var tests = []struct {
command string
additionalFlags []string
}{
{
command: "all",
additionalFlags: []string{
"apiserver-advertise-address",
"apiserver-bind-port",
"pod-network-cidr",
"service-dns-domain",
},
},
{
command: "kube-proxy",
additionalFlags: []string{
"apiserver-advertise-address",
"apiserver-bind-port",
"pod-network-cidr",
},
},
{
command: "kube-dns",
additionalFlags: []string{
"service-dns-domain",
},
},
}

for _, test := range tests {
expectedFlags := append(commonFlags, test.additionalFlags...)
cmdtestutil.AssertSubCommandHasFlags(t, subCmds, test.command, expectedFlags...)
}
}
1 change: 1 addition & 0 deletions cmd/kubeadm/app/cmd/phases/phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func NewCmdPhase(out io.Writer) *cobra.Command {
RunE: cmdutil.SubCmdRunE("phase"),
}

cmd.AddCommand(NewCmdAddon())
cmd.AddCommand(NewCmdBootstrapToken())
cmd.AddCommand(NewCmdCerts())
cmd.AddCommand(NewCmdControlplane())
Expand Down
7 changes: 6 additions & 1 deletion cmd/kubeadm/app/phases/addons/dns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,12 @@ const (
)

// EnsureDNSAddon creates the kube-dns addon
func EnsureDNSAddon(cfg *kubeadmapi.MasterConfiguration, client clientset.Interface, k8sVersion *version.Version) error {
func EnsureDNSAddon(cfg *kubeadmapi.MasterConfiguration, client clientset.Interface) error {
k8sVersion, err := version.ParseSemantic(cfg.KubernetesVersion)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to pass k8sVersion still and do the parsing client-side in cmd/phases
Luckily, I expect to soon get rid of this in the v1.9 cycle by moving this into the internal variant of cfg or something so we can stop passing and parsing this all the time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only issue I had with this was that I needed all the EnsureXXXAddon functions have the same signature. Look at the EnsureAllAddons and runAddonsCmdFunc functions. I'm happy to do something different if we still prefer to pass k8sVersion in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok with this for now; will fix it for real later

if err != nil {
return fmt.Errorf("couldn't parse kubernetes version %q: %v", cfg.KubernetesVersion, err)
}

if err := CreateServiceAccount(client); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/phases/upgrade/postupgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.MasterC
}

// Upgrade kube-dns and kube-proxy
if err := dns.EnsureDNSAddon(cfg, client, k8sVersion); err != nil {
if err := dns.EnsureDNSAddon(cfg, client); err != nil {
errs = append(errs, err)
}
if err := proxy.EnsureProxyAddon(cfg, client); err != nil {
Expand Down