Skip to content

Commit

Permalink
kubeadm: ensure the kubelet and kube-apiserver wait checks go first
Browse files Browse the repository at this point in the history 
The addition of the "super-admin.conf" functionality required
init.go's Client() to create RBAC rules on its first creation.

However this created a problem with the "wait-control-plane" phase
of "kubeadm init" where a client is needed to connect to the
API server Discovery API's "/healthz" endpoint. The logic that ensures
the RBAC became the step where the API server wait was polled for.

To avoid this, introduce a new InitData function ClientWithoutBootstrap.
In "wait-control-plane" use this client, which has no permissions
(anonymous), but is sufficient to connect to the "/healthz".

Pending changes here would be:
- Stop using the "/healthz", instead a regular REST client from
the kubelet cert/key can be constructed.
- Make the wait for kubelet / API server linear (not in go routines).
  • Loading branch information
@neolit123
neolit123 committed Nov 6, 2023
1 parent 24e6b03 commit 6dc11c1
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 27 deletions.
41 changes: 35 additions & 6 deletions cmd/kubeadm/app/cmd/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,21 +496,28 @@ func (d *initData) OutputWriter() io.Writer {
return d.outputWriter
}

// getDryRunClient creates a fake client that answers some GET calls in order to be able to do the full init flow in dry-run mode.
func getDryRunClient(d *initData) (clientset.Interface, error) {
svcSubnetCIDR, err := kubeadmconstants.GetKubernetesServiceCIDR(d.cfg.Networking.ServiceSubnet)
if err != nil {
return nil, errors.Wrapf(err, "unable to get internal Kubernetes Service IP from the given service CIDR (%s)", d.cfg.Networking.ServiceSubnet)
}
dryRunGetter := apiclient.NewInitDryRunGetter(d.cfg.NodeRegistration.Name, svcSubnetCIDR.String())
return apiclient.NewDryRunClient(dryRunGetter, os.Stdout), nil
}

// Client returns a Kubernetes client to be used by kubeadm.
// This function is implemented as a singleton, thus avoiding to recreate the client when it is used by different phases.
// Important. This function must be called after the admin.conf kubeconfig file is created.
func (d *initData) Client() (clientset.Interface, error) {
var err error
if d.client == nil {
if d.dryRun {
svcSubnetCIDR, err := kubeadmconstants.GetKubernetesServiceCIDR(d.cfg.Networking.ServiceSubnet)
d.client, err = getDryRunClient(d)
if err != nil {
return nil, errors.Wrapf(err, "unable to get internal Kubernetes Service IP from the given service CIDR (%s)", d.cfg.Networking.ServiceSubnet)
return nil, err
}
// If we're dry-running, we should create a faked client that answers some GETs in order to be able to do the full init flow and just logs the rest of requests
dryRunGetter := apiclient.NewInitDryRunGetter(d.cfg.NodeRegistration.Name, svcSubnetCIDR.String())
d.client = apiclient.NewDryRunClient(dryRunGetter, os.Stdout)
} else { // Use a real client
var err error
if !d.adminKubeConfigBootstrapped {
// Call EnsureAdminClusterRoleBinding() to obtain a working client from admin.conf.
d.client, err = kubeconfigphase.EnsureAdminClusterRoleBinding(kubeadmconstants.KubernetesDir, nil)
Expand All @@ -531,6 +538,28 @@ func (d *initData) Client() (clientset.Interface, error) {
return d.client, nil
}

// ClientWithoutBootstrap returns a dry-run client or a regular client from admin.conf.
// Unlike Client(), it does not call EnsureAdminClusterRoleBinding() or sets d.client.
// This means the client only has anonymous permissions and does not persist in initData.
func (d *initData) ClientWithoutBootstrap() (clientset.Interface, error) {
var (
client clientset.Interface
err error
)
if d.dryRun {
client, err = getDryRunClient(d)
if err != nil {
return nil, err
}
} else { // Use a real client
client, err = kubeconfigutil.ClientSetFromFile(d.KubeConfigPath())
if err != nil {
return nil, err
}
}
return client, nil
}

// Tokens returns an array of token strings.
func (d *initData) Tokens() []string {
tokens := []string{}
Expand Down
1 change: 1 addition & 0 deletions cmd/kubeadm/app/cmd/phases/init/data.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type InitData interface {
ExternalCA() bool
OutputWriter() io.Writer
Client() (clientset.Interface, error)
ClientWithoutBootstrap() (clientset.Interface, error)
Tokens() []string
PatchesDir() string
}
39 changes: 20 additions & 19 deletions cmd/kubeadm/app/cmd/phases/init/data_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,23 @@ type testInitData struct{}
// testInitData must satisfy InitData.
var _ InitData = &testInitData{}

func (t *testInitData) UploadCerts() bool { return false }
func (t *testInitData) CertificateKey() string { return "" }
func (t *testInitData) SetCertificateKey(key string) {}
func (t *testInitData) SkipCertificateKeyPrint() bool { return false }
func (t *testInitData) Cfg() *kubeadmapi.InitConfiguration { return nil }
func (t *testInitData) DryRun() bool { return false }
func (t *testInitData) SkipTokenPrint() bool { return false }
func (t *testInitData) IgnorePreflightErrors() sets.Set[string] { return nil }
func (t *testInitData) CertificateWriteDir() string { return "" }
func (t *testInitData) CertificateDir() string { return "" }
func (t *testInitData) KubeConfigDir() string { return "" }
func (t *testInitData) KubeConfigPath() string { return "" }
func (t *testInitData) ManifestDir() string { return "" }
func (t *testInitData) KubeletDir() string { return "" }
func (t *testInitData) ExternalCA() bool { return false }
func (t *testInitData) OutputWriter() io.Writer { return nil }
func (t *testInitData) Client() (clientset.Interface, error) { return nil, nil }
func (t *testInitData) Tokens() []string { return nil }
func (t *testInitData) PatchesDir() string { return "" }
func (t *testInitData) UploadCerts() bool { return false }
func (t *testInitData) CertificateKey() string { return "" }
func (t *testInitData) SetCertificateKey(key string) {}
func (t *testInitData) SkipCertificateKeyPrint() bool { return false }
func (t *testInitData) Cfg() *kubeadmapi.InitConfiguration { return nil }
func (t *testInitData) DryRun() bool { return false }
func (t *testInitData) SkipTokenPrint() bool { return false }
func (t *testInitData) IgnorePreflightErrors() sets.Set[string] { return nil }
func (t *testInitData) CertificateWriteDir() string { return "" }
func (t *testInitData) CertificateDir() string { return "" }
func (t *testInitData) KubeConfigDir() string { return "" }
func (t *testInitData) KubeConfigPath() string { return "" }
func (t *testInitData) ManifestDir() string { return "" }
func (t *testInitData) KubeletDir() string { return "" }
func (t *testInitData) ExternalCA() bool { return false }
func (t *testInitData) OutputWriter() io.Writer { return nil }
func (t *testInitData) Client() (clientset.Interface, error) { return nil, nil }
func (t *testInitData) ClientWithoutBootstrap() (clientset.Interface, error) { return nil, nil }
func (t *testInitData) Tokens() []string { return nil }
func (t *testInitData) PatchesDir() string { return "" }
5 changes: 3 additions & 2 deletions cmd/kubeadm/app/cmd/phases/init/waitcontrolplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,10 @@ func runWaitControlPlanePhase(c workflow.RunData) error {
// waiter holds the apiclient.Waiter implementation of choice, responsible for querying the API server in various ways and waiting for conditions to be fulfilled
klog.V(1).Infoln("[wait-control-plane] Waiting for the API server to be healthy")

client, err := data.Client()
// WaitForAPI uses the /healthz endpoint, thus a client without permissions works fine
client, err := data.ClientWithoutBootstrap()
if err != nil {
return errors.Wrap(err, "cannot obtain client")
return errors.Wrap(err, "cannot obtain client without bootstrap")
}

timeout := data.Cfg().ClusterConfiguration.APIServer.TimeoutForControlPlane.Duration
Expand Down

0 comments on commit 6dc11c1

Please sign in to comment.