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

Fix multi-node cluster not working after restarting docker #2671

Closed
wants to merge 1 commit into from
Closed
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
66 changes: 59 additions & 7 deletions pkg/cluster/internal/create/actions/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"sigs.k8s.io/kind/pkg/cluster/internal/providers/common"
"sigs.k8s.io/kind/pkg/cluster/nodeutils"
"sigs.k8s.io/kind/pkg/internal/apis/config"
"sigs.k8s.io/kind/pkg/internal/version"
)

// Action implements action for creating the node config files
Expand Down Expand Up @@ -87,6 +88,23 @@ func (a *Action) Execute(ctx *actions.ActionContext) error {
kubeadmConfigPlusPatches := func(node nodes.Node, data kubeadm.ConfigData) func() error {
return func() error {
data.NodeName = node.String()
kubeVersion, err := nodeutils.KubeVersion(node)
if err != nil {
// TODO(bentheelder): logging here
return errors.Wrap(err, "failed to get kubernetes version from node")
}
data.KubernetesVersion = kubeVersion

patches, err := getKubeadmPatches(data)
if err != nil {
return errors.Wrap(err, "failed to generate kubeadm patches content")
}

ctx.Logger.V(2).Infof("Using the following kubeadm patches for node %s:\n%s", node.String(), patches)
if err := writeKubeadmPatches(patches, node); err != nil {
return err
}

kubeadmConfig, err := getKubeadmConfig(ctx.Config, data, node, provider)
if err != nil {
// TODO(bentheelder): logging here
Expand Down Expand Up @@ -155,13 +173,6 @@ func (a *Action) Execute(ctx *actions.ActionContext) error {
// getKubeadmConfig generates the kubeadm config contents for the cluster
// by running data through the template and applying patches as needed.
func getKubeadmConfig(cfg *config.Cluster, data kubeadm.ConfigData, node nodes.Node, provider string) (path string, err error) {
kubeVersion, err := nodeutils.KubeVersion(node)
if err != nil {
// TODO(bentheelder): logging here
return "", errors.Wrap(err, "failed to get kubernetes version from node")
}
data.KubernetesVersion = kubeVersion

// TODO: gross hack!
// identify node in config by matching name (since these are named in order)
// we should really just streamline the bootstrap code and maintain
Expand Down Expand Up @@ -241,6 +252,31 @@ func getKubeadmConfig(cfg *config.Cluster, data kubeadm.ConfigData, node nodes.N
return removeMetadata(patchedConfig), nil
}

// getKubeadmPatches generates the kubeadm patch contents. It returns a map of patch file name to patch content.
func getKubeadmPatches(data kubeadm.ConfigData) (map[string]string, error) {
ver, err := version.ParseGeneric(data.KubernetesVersion)
if err != nil {
return nil, err
}

patches := map[string]string{}
// Kubernetes older than v1.25 don't support patching kubeconfig files.
if ver.AtLeast(version.MustParseSemantic("v1.25.0")) {
Copy link
Member

Choose a reason for hiding this comment

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

we'll want to make this more precise to the version that supports this, so we can start testing it sooner. we can get a version after merge if it merges with hack/print-workspace-status.sh

// controller-manager and scheduler connect to local API endpoint, which defaults to the advertise address of the
// API server. If the Node's IP changes (which could happen after docker restarts), the server address in KubeConfig
// should be updated. However, the server certificate isn't valid for the new IP. To resolve it, we update the
Copy link
Member

Choose a reason for hiding this comment

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

we should probably fix this, perhaps in the entrypoint we can re-roll the cert if it exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. It reminds me that since we have the CA cert and key, we could just re-roll the cert and replace the server address in kubeconfig with the new Node IP like the existing magic:

# kubernetes manifests are only present on control-plane nodes
sed -i "s#${old_ipv4}#${curr_ipv4}#" /etc/kubernetes/manifests/*.yaml || true

Then everything should just work in theory. Do you think this is a better solution?

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 tried the above approach and it worked as expected, created issue #2759 to discuss the approaches.

// address to loopback address which is an alternative address of the certificate.
loopbackAddress := "127.0.0.1"
if data.IPFamily == config.IPv6Family {
loopbackAddress = "[::1]"
}
jsonPatch := fmt.Sprintf(`[{"op": "replace", "path": "/clusters/0/cluster/server", "value": "https://%s:%d"}]`, loopbackAddress, data.APIBindPort)
Comment on lines +269 to +273
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
loopbackAddress := "127.0.0.1"
if data.IPFamily == config.IPv6Family {
loopbackAddress = "[::1]"
}
jsonPatch := fmt.Sprintf(`[{"op": "replace", "path": "/clusters/0/cluster/server", "value": "https://%s:%d"}]`, loopbackAddress, data.APIBindPort)
loopbackAddress := "127.0.0.1"
if data.IPFamily == config.IPv6Family {
loopbackAddress = "::1"
}
jsonPatch := fmt.Sprintf(`[{"op": "replace", "path": "/clusters/0/cluster/server", "value": "https://%s"}]`, net.JoinHostPort(loopbackAddress, strconv.Itoa(data.APIBindPort)))

patches["controller-manager.conf+json.json"] = jsonPatch
patches["scheduler.conf+json.json"] = jsonPatch
}
return patches, nil
}

// trims out the metadata.name we put in the config for kustomize matching,
// kubeadm will complain about this otherwise
func removeMetadata(kustomized string) string {
Expand Down Expand Up @@ -269,6 +305,22 @@ func writeKubeadmConfig(kubeadmConfig string, node nodes.Node) error {
return nil
}

// writeKubeadmPatches writes the kubeadm patches in the specified node
func writeKubeadmPatches(patches map[string]string, node nodes.Node) error {
patchesDir := "/kind/patches/"
if err := node.Command("mkdir", "-p", patchesDir).Run(); err != nil {
return errors.Wrapf(err, "failed to create directory %s", patchesDir)
}

for file, patch := range patches {
if err := nodeutils.WriteFile(node, patchesDir+file, patch); err != nil {
return errors.Wrapf(err, "failed to copy patch file %s to node", file)
}
}

return nil
}

// hashMapLabelsToCommaSeparatedLabels converts labels in hashmap form to labels in a comma-separated string form like "key1=value1,key2=value2"
func hashMapLabelsToCommaSeparatedLabels(labels map[string]string) string {
output := ""
Expand Down
4 changes: 4 additions & 0 deletions pkg/cluster/internal/kubeadm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,8 @@ apiVersion: kubeadm.k8s.io/v1beta3
kind: InitConfiguration
metadata:
name: config
patches:
directory: /kind/patches
# we use a well know token for TLS bootstrap
bootstrapTokens:
- token: "{{ .Token }}"
Expand All @@ -498,6 +500,8 @@ apiVersion: kubeadm.k8s.io/v1beta3
kind: JoinConfiguration
metadata:
name: config
patches:
directory: /kind/patches
{{ if .ControlPlane -}}
controlPlane:
localAPIEndpoint:
Expand Down