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

set skipPhases in Init and JoinConfiguration #3624

Merged
merged 1 commit into from
May 21, 2024
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
34 changes: 24 additions & 10 deletions pkg/cluster/internal/create/actions/kubeadminit/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,38 @@ func (a *action) Execute(ctx *actions.ActionContext) error {
return err
}

// skip preflight checks, as these have undesirable side effects
// and don't tell us much. requires kubeadm 1.13+
skipPhases := "preflight"
if a.skipKubeProxy {
skipPhases += ",addon/kube-proxy"
kubeVersionStr, err := nodeutils.KubeVersion(node)
if err != nil {
return errors.Wrap(err, "failed to get kubernetes version from node")
}
kubeVersion, err := version.ParseGeneric(kubeVersionStr)
if err != nil {
return errors.Wrapf(err, "failed to parse kubernetes version %q", kubeVersionStr)
}

// run kubeadm
cmd := node.Command(
args := []string{
// init because this is the control plane node
"kubeadm", "init",
"--skip-phases="+skipPhases,
"init",
// specify our generated config file
"--config=/kind/kubeadm.conf",
"--skip-token-print",
// increase verbosity for debugging
"--v=6",
)
}

// Newer versions set this in the config file.
if kubeVersion.LessThan(version.MustParseSemantic("v1.23.0")) {
// skip preflight checks, as these have undesirable side effects
// and don't tell us much. requires kubeadm 1.13+
skipPhases := "preflight"
if a.skipKubeProxy {
skipPhases += ",addon/kube-proxy"
}
args = append(args, "--skip-phases="+skipPhases)
}

// run kubeadm
cmd := node.Command("kubeadm", args...)
lines, err := exec.CombinedOutputLines(cmd)
ctx.Logger.V(3).Info(strings.Join(lines, "\n"))
if err != nil {
Expand Down
30 changes: 22 additions & 8 deletions pkg/cluster/internal/create/actions/kubeadmjoin/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"sigs.k8s.io/kind/pkg/cluster/nodes"
"sigs.k8s.io/kind/pkg/errors"
"sigs.k8s.io/kind/pkg/exec"
"sigs.k8s.io/kind/pkg/internal/version"
"sigs.k8s.io/kind/pkg/log"

"sigs.k8s.io/kind/pkg/cluster/nodeutils"
Expand Down Expand Up @@ -117,18 +118,31 @@ func joinWorkers(

// runKubeadmJoin executes kubeadm join command
func runKubeadmJoin(logger log.Logger, node nodes.Node) error {
// run kubeadm join
// TODO(bentheelder): this should be using the config file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BenTheElder It seemed like this was the last missing piece for this TODO. Please let me know if there's still a gap here where this comment is still valuable.

Copy link
Member

Choose a reason for hiding this comment

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

yep! thanks for fixing my ancient TODO 😅

cmd := node.Command(
"kubeadm", "join",
kubeVersionStr, err := nodeutils.KubeVersion(node)
if err != nil {
return errors.Wrap(err, "failed to get kubernetes version from node")
}
kubeVersion, err := version.ParseGeneric(kubeVersionStr)
if err != nil {
return errors.Wrapf(err, "failed to parse kubernetes version %q", kubeVersionStr)
}

args := []string{
"join",
// the join command uses the config file generated in a well known location
"--config", "/kind/kubeadm.conf",
// skip preflight checks, as these have undesirable side effects
// and don't tell us much. requires kubeadm 1.13+
"--skip-phases=preflight",
// increase verbosity for debugging
"--v=6",
)
}
// Newer versions set this in the config file.
if kubeVersion.LessThan(version.MustParseSemantic("v1.23.0")) {
// skip preflight checks, as these have undesirable side effects
// and don't tell us much. requires kubeadm 1.13+
args = append(args, "--skip-phases=preflight")
}

// run kubeadm join
cmd := node.Command("kubeadm", args...)
lines, err := exec.CombinedOutputLines(cmd)
logger.V(3).Info(strings.Join(lines, "\n"))
if err != nil {
Expand Down
24 changes: 24 additions & 0 deletions pkg/cluster/internal/kubeadm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ type DerivedConfigData struct {
IPv6 bool
// kubelet cgroup driver, based on kubernetes version
CgroupDriver string
// JoinSkipPhases are the skipPhases values for the JoinConfiguration.
JoinSkipPhases []string
// InitSkipPhases are the skipPhases values for the InitConfiguration.
InitSkipPhases []string
BenTheElder marked this conversation as resolved.
Show resolved Hide resolved
}

type FeatureGate struct {
Expand Down Expand Up @@ -166,6 +170,14 @@ func (c *ConfigData) Derive() {
runtimeConfig = append(runtimeConfig, fmt.Sprintf("%s=%s", k, v))
}
c.RuntimeConfigString = strings.Join(runtimeConfig, ",")

// skip preflight checks, as these have undesirable side effects
Copy link
Member

Choose a reason for hiding this comment

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

TODO(bentheelder): write a fairer note here. this old comment kinda reads like they're a useless feature and that's not accurate, it's just not a good match for kind (we pre-pull images ourselves, we intentionally run in environments with swap on, etc. I wouldn't skip these if running kubeadm init directly on a linux host.

Copy link
Member

Choose a reason for hiding this comment

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

(also the 1.13+ comment is now redundant, we stopped supporting Kubernetes < 1.15 in https://github.com/kubernetes-sigs/kind/releases/tag/v0.16.0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can rewrite those old comments in a follow-up

@BenTheElder Sorry if you were still getting to this, but I took the liberty of making these changes here: #3632

// and don't tell us much. requires kubeadm 1.22+
c.JoinSkipPhases = []string{"preflight"}
c.InitSkipPhases = []string{"preflight"}
if c.KubeProxyMode == string(config.NoneProxyMode) {
c.InitSkipPhases = append(c.InitSkipPhases, "addon/kube-proxy")
}
}

// See docs for these APIs at:
Expand Down Expand Up @@ -380,6 +392,12 @@ nodeRegistration:
node-ip: "{{ .NodeAddress }}"
provider-id: "kind://{{.NodeProvider}}/{{.ClusterName}}/{{.NodeName}}"
node-labels: "{{ .NodeLabels }}"
{{ if .InitSkipPhases -}}
skipPhases:
{{ range $phase := .InitSkipPhases -}}
- "{{ $phase }}"
{{- end }}
{{- end }}
---
# no-op entry that exists solely so it can be patched
apiVersion: kubeadm.k8s.io/v1beta3
Expand All @@ -403,6 +421,12 @@ discovery:
apiServerEndpoint: "{{ .ControlPlaneEndpoint }}"
token: "{{ .Token }}"
unsafeSkipCAVerification: true
{{ if .JoinSkipPhases -}}
skipPhases:
{{ range $phase := .JoinSkipPhases -}}
- "{{ $phase }}"
{{- end }}
{{- end }}
---
apiVersion: kubelet.config.k8s.io/v1beta1
kind: KubeletConfiguration
Expand Down
Loading