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

Implement individual control for kubeadm preflight checks #56072

Merged
merged 1 commit into from
Nov 22, 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
1 change: 1 addition & 0 deletions cmd/kubeadm/app/apis/kubeadm/validation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ go_library(
"//pkg/registry/core/service/ipallocator:go_default_library",
"//pkg/util/node:go_default_library",
"//vendor/github.com/spf13/pflag:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
Expand Down
27 changes: 26 additions & 1 deletion cmd/kubeadm/app/apis/kubeadm/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/spf13/pflag"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
Expand Down Expand Up @@ -289,7 +290,7 @@ func ValidateMixedArguments(flag *pflag.FlagSet) error {

mixedInvalidFlags := []string{}
flag.Visit(func(f *pflag.Flag) {
if f.Name == "config" || strings.HasPrefix(f.Name, "skip-") || f.Name == "dry-run" || f.Name == "kubeconfig" {
if f.Name == "config" || strings.HasPrefix(f.Name, "ignore-checks-") || strings.HasPrefix(f.Name, "skip-") || f.Name == "dry-run" || f.Name == "kubeconfig" {
// "--skip-*" flags or other whitelisted flags can be set with --config
return
}
Expand Down Expand Up @@ -328,3 +329,27 @@ func ValidateAPIEndpoint(c *kubeadm.MasterConfiguration, fldPath *field.Path) fi
}
return allErrs
}

// ValidateIgnoreChecksErrors validates duplicates in ignore-checks-errors flag.
func ValidateIgnoreChecksErrors(ignoreChecksErrors []string, skipPreflightChecks bool) (sets.String, error) {
Copy link
Member

Choose a reason for hiding this comment

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

ValidateIgnoreChecksErrors - doesn't parse well when reading.

ignoreErrors := sets.NewString()
allErrs := field.ErrorList{}

for _, item := range ignoreChecksErrors {
ignoreErrors.Insert(strings.ToLower(item)) // parameters are case insensitive
}

// TODO: remove once deprecated flag --skip-preflight-checks is removed.
if skipPreflightChecks {
if ignoreErrors.Has("all") {
allErrs = append(allErrs, field.Invalid(field.NewPath("ignore-checks-errors"), strings.Join(ignoreErrors.List(), ","), "'all' is used together with deprecated flag --skip-preflight-checks. Remove deprecated flag"))
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 not certain...but this looks like it would force an exit if both are specified why couldn't it prefer 'ignore-checks-errors' and warn?

Copy link
Member

Choose a reason for hiding this comment

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

+1. I'd only warn if skipPreflightChecks is there and that's it

}
ignoreErrors.Insert("all")
}

if ignoreErrors.Has("all") && ignoreErrors.Len() > 1 {
allErrs = append(allErrs, field.Invalid(field.NewPath("ignore-checks-errors"), strings.Join(ignoreErrors.List(), ","), "don't specify individual checks if 'all' is used"))
Copy link
Member

Choose a reason for hiding this comment

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

same.. could warn

}

return ignoreErrors, allErrs.ToAggregate()
}
29 changes: 29 additions & 0 deletions cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,3 +458,32 @@ func TestValidateFeatureGates(t *testing.T) {
}
}
}

func TestValidateIgnoreChecksErrors(t *testing.T) {
var tests = []struct {
ignoreChecksErrors []string
skipPreflightChecks bool
expectedLen int
expectedError bool
}{
{[]string{}, false, 0, false}, // empty list, no old skip-preflight-checks
{[]string{}, true, 1, false}, // empty list, old skip-preflight-checks
{[]string{"check1", "check2"}, false, 2, false}, // non-duplicate
{[]string{"check1", "check2"}, true, 3, true}, // non-duplicate, but skip-preflight-checks
{[]string{"check1", "check2", "check1"}, false, 2, false}, // duplicates
{[]string{"check1", "check2", "all"}, false, 3, true}, // non-duplicate, but 'all' present together wth individual checks
{[]string{"all"}, false, 1, false}, // skip all checks by using new flag
{[]string{"all"}, true, 1, true}, // skip all checks by using both old and new flags at the same time
}
for _, rt := range tests {
result, err := ValidateIgnoreChecksErrors(rt.ignoreChecksErrors, rt.skipPreflightChecks)
switch {
case err != nil && !rt.expectedError:
t.Errorf("ValidateIgnoreChecksErrors: unexpected error for input (%s, %v), error: %v", rt.ignoreChecksErrors, rt.skipPreflightChecks, err)
case err == nil && rt.expectedError:
t.Errorf("ValidateIgnoreChecksErrors: expected error for input (%s, %v) but got: %v", rt.ignoreChecksErrors, rt.skipPreflightChecks, result)
case result.Len() != rt.expectedLen:
t.Errorf("ValidateIgnoreChecksErrors: expected Len = %d for input (%s, %v) but got: %v, %v", rt.expectedLen, rt.ignoreChecksErrors, rt.skipPreflightChecks, result.Len(), result)
}
}
}
34 changes: 20 additions & 14 deletions cmd/kubeadm/app/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
flag "github.com/spf13/pflag"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
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"
Expand Down Expand Up @@ -112,6 +113,7 @@ func NewCmdInit(out io.Writer) *cobra.Command {
var dryRun bool
var featureGatesString string
var criSocket string
var ignoreChecksErrors []string

cmd := &cobra.Command{
Use: "init",
Expand All @@ -126,15 +128,18 @@ func NewCmdInit(out io.Writer) *cobra.Command {
internalcfg := &kubeadmapi.MasterConfiguration{}
legacyscheme.Scheme.Convert(cfg, internalcfg, nil)

i, err := NewInit(cfgPath, internalcfg, skipPreFlight, skipTokenPrint, dryRun, criSocket)
ignoreChecksErrorsSet, err := validation.ValidateIgnoreChecksErrors(ignoreChecksErrors, skipPreFlight)
kubeadmutil.CheckErr(err)

i, err := NewInit(cfgPath, internalcfg, ignoreChecksErrorsSet, skipTokenPrint, dryRun, criSocket)
kubeadmutil.CheckErr(err)
kubeadmutil.CheckErr(i.Validate(cmd))
kubeadmutil.CheckErr(i.Run(out))
},
}

AddInitConfigFlags(cmd.PersistentFlags(), cfg, &featureGatesString)
AddInitOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &skipTokenPrint, &dryRun, &criSocket)
AddInitOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &skipTokenPrint, &dryRun, &criSocket, &ignoreChecksErrors)

return cmd
}
Expand Down Expand Up @@ -190,16 +195,21 @@ func AddInitConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.MasterConfigur
}

// AddInitOtherFlags adds init flags that are not bound to a configuration file to the given flagset
func AddInitOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight, skipTokenPrint, dryRun *bool, criSocket *string) {
func AddInitOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight, skipTokenPrint, dryRun *bool, criSocket *string, ignoreChecksErrors *[]string) {
flagSet.StringVar(
cfgPath, "config", *cfgPath,
"Path to kubeadm config file. WARNING: Usage of a configuration file is experimental.",
)
flagSet.StringSliceVar(
ignoreChecksErrors, "ignore-checks-errors", *ignoreChecksErrors,
Copy link
Member

Choose a reason for hiding this comment

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

again ignore-checks-errors is difficult to read and rationalize.

"A list of checks whose errors will be shown as warnings. Example: 'IsPrivilegedUser,Swap'. Value 'all' ignores errors from all checks.",
)
// Note: All flags that are not bound to the cfg object should be whitelisted in cmd/kubeadm/app/apis/kubeadm/validation/validation.go
flagSet.BoolVar(
skipPreFlight, "skip-preflight-checks", *skipPreFlight,
"Skip preflight checks which normally run before modifying the system.",
)
flagSet.MarkDeprecated("skip-preflight-checks", "it is now equivalent to --ignore-checks-errors=all")
// Note: All flags that are not bound to the cfg object should be whitelisted in cmd/kubeadm/app/apis/kubeadm/validation/validation.go
flagSet.BoolVar(
skipTokenPrint, "skip-token-print", *skipTokenPrint,
Expand All @@ -217,7 +227,7 @@ func AddInitOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight, sk
}

// NewInit validates given arguments and instantiates Init struct with provided information.
func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, skipPreFlight, skipTokenPrint, dryRun bool, criSocket string) (*Init, error) {
func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, ignoreChecksErrors sets.String, skipTokenPrint, dryRun bool, criSocket string) (*Init, error) {
fmt.Println("[kubeadm] WARNING: kubeadm is currently in beta")

if cfgPath != "" {
Expand Down Expand Up @@ -249,19 +259,15 @@ func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, skipPreFlight,
fmt.Println("\t(/etc/systemd/system/kubelet.service.d/10-kubeadm.conf should be edited for this purpose)")
}

if !skipPreFlight {
fmt.Println("[preflight] Running pre-flight checks.")
fmt.Println("[preflight] Running pre-flight checks.")

if err := preflight.RunInitMasterChecks(utilsexec.New(), cfg, criSocket); err != nil {
return nil, err
}

// Try to start the kubelet service in case it's inactive
preflight.TryStartKubelet()
} else {
fmt.Println("[preflight] Skipping pre-flight checks.")
if err := preflight.RunInitMasterChecks(utilsexec.New(), cfg, criSocket, ignoreChecksErrors); err != nil {
return nil, err
}

// Try to start the kubelet service in case it's inactive
preflight.TryStartKubelet(ignoreChecksErrors)

return &Init{cfg: cfg, skipTokenPrint: skipTokenPrint, dryRun: dryRun}, nil
}

Expand Down
36 changes: 21 additions & 15 deletions cmd/kubeadm/app/cmd/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
flag "github.com/spf13/pflag"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
clientset "k8s.io/client-go/kubernetes"
certutil "k8s.io/client-go/util/cert"
Expand Down Expand Up @@ -110,6 +111,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
var cfgPath string
var criSocket string
var featureGatesString string
var ignoreChecksErrors []string

cmd := &cobra.Command{
Use: "join [flags]",
Expand All @@ -127,15 +129,18 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
internalcfg := &kubeadmapi.NodeConfiguration{}
legacyscheme.Scheme.Convert(cfg, internalcfg, nil)

j, err := NewJoin(cfgPath, args, internalcfg, skipPreFlight, criSocket)
ignoreChecksErrorsSet, err := validation.ValidateIgnoreChecksErrors(ignoreChecksErrors, skipPreFlight)
kubeadmutil.CheckErr(err)

j, err := NewJoin(cfgPath, args, internalcfg, ignoreChecksErrorsSet, criSocket)
kubeadmutil.CheckErr(err)
kubeadmutil.CheckErr(j.Validate(cmd))
kubeadmutil.CheckErr(j.Run(out))
},
}

AddJoinConfigFlags(cmd.PersistentFlags(), cfg, &featureGatesString)
AddJoinOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &criSocket)
AddJoinOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &criSocket, &ignoreChecksErrors)

return cmd
}
Expand Down Expand Up @@ -170,15 +175,20 @@ func AddJoinConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.NodeConfigurat
}

// AddJoinOtherFlags adds join flags that are not bound to a configuration file to the given flagset
func AddJoinOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight *bool, criSocket *string) {
func AddJoinOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight *bool, criSocket *string, ignoreChecksErrors *[]string) {
flagSet.StringVar(
cfgPath, "config", *cfgPath,
"Path to kubeadm config file.")

flagSet.StringSliceVar(
ignoreChecksErrors, "ignore-checks-errors", *ignoreChecksErrors,
"A list of checks whose errors will be shown as warnings. Example: 'IsPrivilegedUser,Swap'. Value 'all' ignores errors from all checks.",
)
flagSet.BoolVar(
skipPreFlight, "skip-preflight-checks", false,
"Skip preflight checks which normally run before modifying the system.",
)
flagSet.MarkDeprecated("skip-preflight-checks", "it is now equivalent to --ignore-checks-errors=all")
flagSet.StringVar(
criSocket, "cri-socket", "/var/run/dockershim.sock",
`Specify the CRI socket to connect to.`,
Expand All @@ -191,7 +201,7 @@ type Join struct {
}

// NewJoin instantiates Join struct with given arguments
func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, skipPreFlight bool, criSocket string) (*Join, error) {
func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, ignoreChecksErrors sets.String, criSocket string) (*Join, error) {
fmt.Println("[kubeadm] WARNING: kubeadm is currently in beta")

if cfg.NodeName == "" {
Expand All @@ -208,20 +218,16 @@ func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, s
}
}

if !skipPreFlight {
fmt.Println("[preflight] Running pre-flight checks.")
fmt.Println("[preflight] Running pre-flight checks.")

// Then continue with the others...
if err := preflight.RunJoinNodeChecks(utilsexec.New(), cfg, criSocket); err != nil {
return nil, err
}

// Try to start the kubelet service in case it's inactive
preflight.TryStartKubelet()
} else {
fmt.Println("[preflight] Skipping pre-flight checks.")
// Then continue with the others...
if err := preflight.RunJoinNodeChecks(utilsexec.New(), cfg, criSocket, ignoreChecksErrors); err != nil {
return nil, err
}

// Try to start the kubelet service in case it's inactive
preflight.TryStartKubelet(ignoreChecksErrors)

return &Join{cfg: cfg}, nil
}

Expand Down
1 change: 1 addition & 0 deletions cmd/kubeadm/app/cmd/phases/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ go_library(
"//pkg/util/normalizer:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/client-go/kubernetes:go_default_library",
"//vendor/k8s.io/utils/exec:go_default_library",
],
Expand Down
5 changes: 3 additions & 2 deletions cmd/kubeadm/app/cmd/phases/preflight.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package phases
import (
"github.com/spf13/cobra"

"k8s.io/apimachinery/pkg/util/sets"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
Expand Down Expand Up @@ -70,7 +71,7 @@ func NewCmdPreFlightMaster() *cobra.Command {
Run: func(cmd *cobra.Command, args []string) {
cfg := &kubeadmapi.MasterConfiguration{}
criSocket := ""
err := preflight.RunInitMasterChecks(utilsexec.New(), cfg, criSocket)
err := preflight.RunInitMasterChecks(utilsexec.New(), cfg, criSocket, sets.NewString())
kubeadmutil.CheckErr(err)
},
}
Expand All @@ -88,7 +89,7 @@ func NewCmdPreFlightNode() *cobra.Command {
Run: func(cmd *cobra.Command, args []string) {
cfg := &kubeadmapi.NodeConfiguration{}
criSocket := ""
err := preflight.RunJoinNodeChecks(utilsexec.New(), cfg, criSocket)
err := preflight.RunJoinNodeChecks(utilsexec.New(), cfg, criSocket, sets.NewString())
kubeadmutil.CheckErr(err)
},
}
Expand Down
26 changes: 17 additions & 9 deletions cmd/kubeadm/app/cmd/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (

"github.com/spf13/cobra"

"k8s.io/apimachinery/pkg/util/sets"
kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
Expand All @@ -45,20 +47,30 @@ func NewCmdReset(out io.Writer) *cobra.Command {
var skipPreFlight bool
var certsDir string
var criSocketPath string
var ignoreChecksErrors []string

cmd := &cobra.Command{
Use: "reset",
Short: "Run this to revert any changes made to this host by 'kubeadm init' or 'kubeadm join'.",
Run: func(cmd *cobra.Command, args []string) {
r, err := NewReset(skipPreFlight, certsDir, criSocketPath)
ignoreChecksErrorsSet, err := validation.ValidateIgnoreChecksErrors(ignoreChecksErrors, skipPreFlight)
kubeadmutil.CheckErr(err)

r, err := NewReset(ignoreChecksErrorsSet, certsDir, criSocketPath)
kubeadmutil.CheckErr(err)
kubeadmutil.CheckErr(r.Run(out))
},
}

cmd.PersistentFlags().StringSliceVar(
&ignoreChecksErrors, "ignore-checks-errors", ignoreChecksErrors,
"A list of checks whose errors will be shown as warnings. Example: 'IsPrivilegedUser,Swap'. Value 'all' ignores errors from all checks.",
)
cmd.PersistentFlags().BoolVar(
&skipPreFlight, "skip-preflight-checks", false,
"Skip preflight checks which normally run before modifying the system.",
)
cmd.PersistentFlags().MarkDeprecated("skip-preflight-checks", "it is now equivalent to --ignore-checks-errors=all")

cmd.PersistentFlags().StringVar(
&certsDir, "cert-dir", kubeadmapiext.DefaultCertificatesDir,
Expand All @@ -80,15 +92,11 @@ type Reset struct {
}

// NewReset instantiate Reset struct
func NewReset(skipPreFlight bool, certsDir, criSocketPath string) (*Reset, error) {
if !skipPreFlight {
fmt.Println("[preflight] Running pre-flight checks.")
func NewReset(ignoreChecksErrors sets.String, certsDir, criSocketPath string) (*Reset, error) {
fmt.Println("[preflight] Running pre-flight checks.")

if err := preflight.RunRootCheckOnly(); err != nil {
return nil, err
}
} else {
fmt.Println("[preflight] Skipping pre-flight checks.")
if err := preflight.RunRootCheckOnly(ignoreChecksErrors); err != nil {
return nil, err
}

return &Reset{
Expand Down
4 changes: 4 additions & 0 deletions cmd/kubeadm/app/cmd/reset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ func (c *fakeDockerChecker) Check() (warnings, errors []error) {
return c.warnings, c.errors
}

func (c *fakeDockerChecker) Name() string {
return "FakeDocker"
}

func newFakeDockerChecker(warnings, errors []error) preflight.Checker {
return &fakeDockerChecker{warnings: warnings, errors: errors}
}
Expand Down
2 changes: 2 additions & 0 deletions cmd/kubeadm/app/cmd/upgrade/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_library(
deps = [
"//cmd/kubeadm/app/apis/kubeadm:go_default_library",
"//cmd/kubeadm/app/apis/kubeadm/v1alpha1:go_default_library",
"//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library",
"//cmd/kubeadm/app/cmd/util:go_default_library",
"//cmd/kubeadm/app/constants:go_default_library",
"//cmd/kubeadm/app/phases/controlplane:go_default_library",
Expand All @@ -27,6 +28,7 @@ go_library(
"//pkg/util/version:go_default_library",
"//vendor/github.com/ghodss/yaml:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/client-go/discovery/fake:go_default_library",
"//vendor/k8s.io/client-go/kubernetes:go_default_library",
],
Expand Down