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

Remove deprecated --require-kubeconfig flag, remove default --kubeconfig value #58367

Merged
merged 2 commits into from
Jan 29, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions cmd/kubelet/app/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import (
// In general, please try to avoid adding flags or configuration fields,
// we already have a confusingly large amount of them.
type KubeletFlags struct {
KubeConfig flag.StringFlag
KubeConfig string
BootstrapKubeconfig string
RotateCertificates bool

Expand Down Expand Up @@ -179,8 +179,6 @@ type KubeletFlags struct {
// schedulable. Won't have any effect if register-node is false.
// DEPRECATED: use registerWithTaints instead
RegisterSchedulable bool
// RequireKubeConfig is deprecated! A valid KubeConfig is now required if --kubeconfig is provided.
RequireKubeConfig bool
// nonMasqueradeCIDR configures masquerading: traffic to IPs outside this range will use IP masquerade.
NonMasqueradeCIDR string
// This flag, if set, instructs the kubelet to keep volumes from terminated pods mounted to the node.
Expand Down Expand Up @@ -213,9 +211,6 @@ func NewKubeletFlags() *KubeletFlags {
}

return &KubeletFlags{
// TODO(#41161:v1.10.0): Remove the default kubeconfig path and --require-kubeconfig.
RequireKubeConfig: false,
KubeConfig: flag.NewStringFlag("/var/lib/kubelet/kubeconfig"),
ContainerRuntimeOptions: *NewContainerRuntimeOptions(),
CertDirectory: "/var/lib/kubelet/pki",
RootDirectory: v1alpha1.DefaultRootDir,
Expand Down Expand Up @@ -309,13 +304,14 @@ func (s *KubeletServer) AddFlags(fs *pflag.FlagSet) {
func (f *KubeletFlags) AddFlags(fs *pflag.FlagSet) {
f.ContainerRuntimeOptions.AddFlags(fs)

fs.Var(&f.KubeConfig, "kubeconfig", "Path to a kubeconfig file, specifying how to connect to the API server.")
Copy link
Member

Choose a reason for hiding this comment

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

leaving --require-kubeconfig bound to a local string and inert would let this PR merge (since it wouldn't matter whether kops passed the flag or not). I'd be ok with leaving it that way for a release (would let 1.10 kubelets come up even if people specified the deprecated flag). update the deprecated message to say the flag doesn't do anything any more and will be removed in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I will add it back soon. Thanks =)

// TODO(#41161:v1.10.0): Remove the default kubeconfig path and --require-kubeconfig.
fs.BoolVar(&f.RequireKubeConfig, "require-kubeconfig", f.RequireKubeConfig, "This flag is no longer necessary. It has been deprecated and will be removed in a future version.")
fs.MarkDeprecated("require-kubeconfig", "You no longer need to use --require-kubeconfig. This will be removed in a future version. Providing --kubeconfig enables API server mode, omitting --kubeconfig enables standalone mode unless --require-kubeconfig=true is also set. In the latter case, the legacy default kubeconfig path will be used until --require-kubeconfig is removed.")
Copy link
Member

Choose a reason for hiding this comment

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

Keep the same MarkDeprecated as long as we are still binding this flag

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done

fs.StringVar(&f.KubeConfig, "kubeconfig", f.KubeConfig, "Path to a kubeconfig file, specifying how to connect to the API server. Providing --kubeconfig enables API server mode, omitting --kubeconfig enables standalone mode.")

fs.MarkDeprecated("experimental-bootstrap-kubeconfig", "Use --bootstrap-kubeconfig")
fs.StringVar(&f.BootstrapKubeconfig, "experimental-bootstrap-kubeconfig", f.BootstrapKubeconfig, "deprecated: use --bootstrap-kubeconfig")
// TODO: when pull-kubernetes-e2e-kops-aws does not need this parameter, delete requireKubeConfig.
var requireKubeConfig bool
fs.BoolVar(&requireKubeConfig, "require-kubeconfig", requireKubeConfig, "This flag is no longer necessary. It has been deprecated and will be removed in a future version.")
fs.MarkDeprecated("require-kubeconfig", "You no longer need to use --require-kubeconfig. This will be removed in a future version. Providing --kubeconfig enables API server mode, omitting --kubeconfig enables standalone mode unless --require-kubeconfig=true is also set. In the latter case, the legacy default kubeconfig path will be used until --require-kubeconfig is removed.")
fs.StringVar(&f.BootstrapKubeconfig, "bootstrap-kubeconfig", f.BootstrapKubeconfig, "Path to a kubeconfig file that will be used to get client certificate for kubelet. "+
"If the file specified by --kubeconfig does not exist, the bootstrap kubeconfig is used to request a client certificate from the API server. "+
"On success, a kubeconfig file referencing the generated client certificate and key is written to the path specified by --kubeconfig. "+
Expand Down
1 change: 0 additions & 1 deletion cmd/kubelet/app/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func newKubeletServerOrDie() *KubeletServer {
}

func cleanFlags(s *KubeletServer) {
s.KubeConfig = utilflag.NewStringFlag(s.KubeConfig.Value())
s.DynamicConfigDir = utilflag.NewStringFlag(s.DynamicConfigDir.Value())
s.KubeletConfigFile = utilflag.NewStringFlag(s.KubeletConfigFile.Value())
}
Expand Down
102 changes: 44 additions & 58 deletions cmd/kubelet/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,7 @@ func run(s *options.KubeletServer, kubeDeps *kubelet.Dependencies) (err error) {

// About to get clients and such, detect standaloneMode
standaloneMode := true
switch {
case s.RequireKubeConfig == true:
standaloneMode = false
glog.Warningf("--require-kubeconfig is deprecated. Set --kubeconfig without using --require-kubeconfig.")
case s.KubeConfig.Provided():
if len(s.KubeConfig) > 0 {
standaloneMode = false
}

Expand Down Expand Up @@ -310,7 +306,7 @@ func run(s *options.KubeletServer, kubeDeps *kubelet.Dependencies) (err error) {
}

if s.BootstrapKubeconfig != "" {
if err := bootstrap.LoadClientCert(s.KubeConfig.Value(), s.BootstrapKubeconfig, s.CertDirectory, nodeName); err != nil {
if err := bootstrap.LoadClientCert(s.KubeConfig, s.BootstrapKubeconfig, s.CertDirectory, nodeName); err != nil {
return err
}
}
Expand All @@ -330,58 +326,52 @@ func run(s *options.KubeletServer, kubeDeps *kubelet.Dependencies) (err error) {
var externalKubeClient clientset.Interface

clientConfig, err := createAPIServerClientConfig(s)
if err != nil {
return fmt.Errorf("invalid kubeconfig: %v", err)
}

var clientCertificateManager certificate.Manager
if err == nil {
if s.RotateCertificates && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletClientCertificate) {
clientCertificateManager, err = kubeletcertificate.NewKubeletClientCertificateManager(s.CertDirectory, nodeName, clientConfig.CertData, clientConfig.KeyData, clientConfig.CertFile, clientConfig.KeyFile)
if err != nil {
return err
}
// we set exitIfExpired to true because we use this client configuration to request new certs - if we are unable
// to request new certs, we will be unable to continue normal operation
if err := kubeletcertificate.UpdateTransport(wait.NeverStop, clientConfig, clientCertificateManager, true); err != nil {
return err
}
}

kubeClient, err = clientset.NewForConfig(clientConfig)
if s.RotateCertificates && utilfeature.DefaultFeatureGate.Enabled(features.RotateKubeletClientCertificate) {
clientCertificateManager, err = kubeletcertificate.NewKubeletClientCertificateManager(s.CertDirectory, nodeName, clientConfig.CertData, clientConfig.KeyData, clientConfig.CertFile, clientConfig.KeyFile)
if err != nil {
glog.Warningf("New kubeClient from clientConfig error: %v", err)
} else if kubeClient.CertificatesV1beta1() != nil && clientCertificateManager != nil {
glog.V(2).Info("Starting client certificate rotation.")
clientCertificateManager.SetCertificateSigningRequestClient(kubeClient.CertificatesV1beta1().CertificateSigningRequests())
clientCertificateManager.Start()
return err
}
externalKubeClient, err = clientset.NewForConfig(clientConfig)
if err != nil {
glog.Warningf("New kubeClient from clientConfig error: %v", err)
// we set exitIfExpired to true because we use this client configuration to request new certs - if we are unable
// to request new certs, we will be unable to continue normal operation
if err := kubeletcertificate.UpdateTransport(wait.NeverStop, clientConfig, clientCertificateManager, true); err != nil {
return err
}
}

// make a separate client for events
eventClientConfig := *clientConfig
eventClientConfig.QPS = float32(s.EventRecordQPS)
eventClientConfig.Burst = int(s.EventBurst)
eventClient, err = v1core.NewForConfig(&eventClientConfig)
if err != nil {
glog.Warningf("Failed to create API Server client for Events: %v", err)
}
kubeClient, err = clientset.NewForConfig(clientConfig)
if err != nil {
glog.Warningf("New kubeClient from clientConfig error: %v", err)
} else if kubeClient.CertificatesV1beta1() != nil && clientCertificateManager != nil {
glog.V(2).Info("Starting client certificate rotation.")
clientCertificateManager.SetCertificateSigningRequestClient(kubeClient.CertificatesV1beta1().CertificateSigningRequests())
clientCertificateManager.Start()
}
externalKubeClient, err = clientset.NewForConfig(clientConfig)
if err != nil {
glog.Warningf("New kubeClient from clientConfig error: %v", err)
}

// make a separate client for heartbeat with throttling disabled and a timeout attached
heartbeatClientConfig := *clientConfig
heartbeatClientConfig.Timeout = s.KubeletConfiguration.NodeStatusUpdateFrequency.Duration
heartbeatClientConfig.QPS = float32(-1)
heartbeatClient, err = v1core.NewForConfig(&heartbeatClientConfig)
if err != nil {
glog.Warningf("Failed to create API Server client for heartbeat: %v", err)
}
} else {
switch {
case s.RequireKubeConfig:
return fmt.Errorf("invalid kubeconfig: %v", err)
case s.KubeConfig.Provided():
glog.Warningf("invalid kubeconfig: %v", err)
}
// make a separate client for events
eventClientConfig := *clientConfig
eventClientConfig.QPS = float32(s.EventRecordQPS)
eventClientConfig.Burst = int(s.EventBurst)
eventClient, err = v1core.NewForConfig(&eventClientConfig)
if err != nil {
glog.Warningf("Failed to create API Server client for Events: %v", err)
}

// make a separate client for heartbeat with throttling disabled and a timeout attached
heartbeatClientConfig := *clientConfig
heartbeatClientConfig.Timeout = s.KubeletConfiguration.NodeStatusUpdateFrequency.Duration
heartbeatClientConfig.QPS = float32(-1)
heartbeatClient, err = v1core.NewForConfig(&heartbeatClientConfig)
if err != nil {
glog.Warningf("Failed to create API Server client for heartbeat: %v", err)
}

kubeDeps.KubeClient = kubeClient
Expand Down Expand Up @@ -594,19 +584,15 @@ func InitializeTLS(kf *options.KubeletFlags, kc *kubeletconfiginternal.KubeletCo

func kubeconfigClientConfig(s *options.KubeletServer) (*restclient.Config, error) {
return clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
&clientcmd.ClientConfigLoadingRules{ExplicitPath: s.KubeConfig.Value()},
&clientcmd.ClientConfigLoadingRules{ExplicitPath: s.KubeConfig},
&clientcmd.ConfigOverrides{},
).ClientConfig()
}

// createClientConfig creates a client configuration from the command line arguments.
// If --kubeconfig is explicitly set, it will be used. If it is not set but
// --require-kubeconfig=true, we attempt to load the default kubeconfig file.
// If --kubeconfig is explicitly set, it will be used.
func createClientConfig(s *options.KubeletServer) (*restclient.Config, error) {
// If --kubeconfig was not provided, it will have a default path set in cmd/kubelet/app/options/options.go.
// We only use that default path when --require-kubeconfig=true. The default path is temporary until --require-kubeconfig is removed.
// TODO(#41161:v1.10.0): Remove the default kubeconfig path and --require-kubeconfig.
if s.BootstrapKubeconfig != "" || s.KubeConfig.Provided() || s.RequireKubeConfig == true {
if s.BootstrapKubeconfig != "" || len(s.KubeConfig) > 0 {
return kubeconfigClientConfig(s)
} else {
return nil, fmt.Errorf("createClientConfig called in standalone mode")
Expand Down