Bug 1931974: Prefer IPv6 hostIP on bootstrap IPv6 deployments #4756

celebdor · 2021-03-16T03:33:22Z

Kubelet can end up choosing IPv4 addresses as its hostIP, which after:

https://github.com/openshift/cluster-kube-apiserver-operator/pull/1042/files

mean that the kubernetes API service ends up with an endpoint being an
IPv4 address. This breaks the kubernetes API loadbalancer in the
service network and prevent the deployment from succeeding.

openshift-ci-robot · 2021-03-16T03:33:29Z

@celebdor: This pull request references Bugzilla bug 1931974, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.8.0) matches configured target release for branch (4.8.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (vvoronko@redhat.com), skipping review request.

Details

In response to this:

Bug 1931974: Prefer IPv6 hostIP on bootstrap IPv6 deployments

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

celebdor · 2021-03-16T03:34:05Z

still need to test it out

pkg/asset/ignition/bootstrap/baremetal/template.go

celebdor · 2021-04-07T01:37:15Z

@staebler that makes it simpler, thanks, It renders well

[asegurap@edge-24 installer]$ ssh core@fd2e:6f44:5dd8:c956::2c
[core@localhost ~]$ cat /etc/systemd/system/kubelet.service 
[Unit]
Description=Kubernetes Kubelet
Wants=rpc-statd.service crio.service release-image.service
After=crio.service release-image.service

[Service]
Type=notify
ExecStartPre=/bin/mkdir --parents /etc/kubernetes/manifests
ExecStartPre=/bin/mkdir --parents /etc/kubernetes/kubelet-plugins/volume/exec
ExecStartPre=/usr/local/bin/kubelet-pause-image.sh
Environment=KUBELET_RUNTIME_REQUEST_TIMEOUT=10m
EnvironmentFile=-/etc/kubernetes/kubelet-env
EnvironmentFile=-/etc/kubernetes/kubelet-pause-image-override

ExecStart=/usr/bin/hyperkube \
  kubelet \
    --anonymous-auth=false \
    --container-runtime=remote \
    --container-runtime-endpoint=/var/run/crio/crio.sock \
    --runtime-request-timeout=${KUBELET_RUNTIME_REQUEST_TIMEOUT} \
    --node-ip=:: \
    --pod-manifest-path=/etc/kubernetes/manifests \
    --minimum-container-ttl-duration=6m0s \
    --cluster-domain=cluster.local \
    --cgroup-driver=systemd \
    --serialize-image-pulls=false \
    --v=2 \
    --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
    --pod-infra-container-image=${MACHINE_CONFIG_INFRA_IMAGE}

Restart=always
RestartSec=10

[Install]
WantedBy=multi-user.target
[core@localhost ~]$

celebdor · 2021-04-07T01:59:32Z

After rebase:

[asegurap@edge-24 installer]$ ssh core@fd2e:6f44:5dd8:c956::33
[core@localhost ~]$ cat /usr/local/bin/kubelet.sh 
#!/usr/bin/env bash

# shellcheck disable=SC1091  # using path on bootstrap machine
. /usr/local/bin/bootstrap-service-record.sh

/usr/bin/hyperkube \
  kubelet \
    --anonymous-auth=false \
    --container-runtime=remote \
    --container-runtime-endpoint=/var/run/crio/crio.sock \
    --runtime-request-timeout="${KUBELET_RUNTIME_REQUEST_TIMEOUT}" \
    --node-ip=:: \
    --pod-manifest-path=/etc/kubernetes/manifests \
    --minimum-container-ttl-duration=6m0s \
    --cluster-domain=cluster.local \
    --cgroup-driver=systemd \
    --serialize-image-pulls=false \
    --v=2 \
    --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
    --pod-infra-container-image="${MACHINE_CONFIG_INFRA_IMAGE}"
[core@localhost ~]$

celebdor · 2021-04-07T02:22:48Z

Moved the kubelet.sh to only be rendered with the ipv6 check on the batemetal platform. Still rendering well:

[asegurap@edge-24 installer]$ ssh core@fd2e:6f44:5dd8:c956::1d cat /usr/local/bin/kubelet.sh
#!/usr/bin/env bash

# shellcheck disable=SC1091  # using path on bootstrap machine
. /usr/local/bin/bootstrap-service-record.sh

/usr/bin/hyperkube \
  kubelet \
    --anonymous-auth=false \
    --container-runtime=remote \
    --container-runtime-endpoint=/var/run/crio/crio.sock \
    --runtime-request-timeout="${KUBELET_RUNTIME_REQUEST_TIMEOUT}" \
    --node-ip=:: \
    --pod-manifest-path=/etc/kubernetes/manifests \
    --minimum-container-ttl-duration=6m0s \
    --cluster-domain=cluster.local \
    --cgroup-driver=systemd \
    --serialize-image-pulls=false \
    --v=2 \
    --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
    --pod-infra-container-image="${MACHINE_CONFIG_INFRA_IMAGE}"
[asegurap@edge-24 installer]$

celebdor · 2021-04-07T08:28:00Z

/retest

osherdp · 2021-04-07T09:59:58Z

/lgtm

osherdp · 2021-04-07T10:50:21Z

I just tested it, it seems to fix the problem
Thanks!

celebdor · 2021-04-07T12:59:59Z

/retest

staebler · 2021-04-07T14:23:09Z

Moved the kubelet.sh to only be rendered with the ipv6 check on the batemetal platform.

I don't think it is a good idea to have a completely separate kubelet.sh file for baremetal. What is the reasoning behind copying the file?

Note that the code creating the bootstrap ignition config adds both the regular kubelet.sh file and the baremetal-specific kubelet.sh file. You are getting the bootstrap-specific one only because the code happens to add that one second: That is not guaranteed.

staebler · 2021-04-07T14:23:51Z

/hold for concerns over duplicate kubelet.sh file

celebdor · 2021-04-14T17:20:36Z

Correctly rendered with the new approach:

[asegurap@edge-24 ~]$ ssh core@fd2e:6f44:5dd8:c956::18 cat /usr/local/bin/kubelet.sh
#!/usr/bin/env bash

# shellcheck disable=SC1091  # using path on bootstrap machine
. /usr/local/bin/bootstrap-service-record.sh

/usr/bin/hyperkube \
  kubelet \
    --anonymous-auth=false \
    --container-runtime=remote \
    --container-runtime-endpoint=/var/run/crio/crio.sock \
    --runtime-request-timeout="${KUBELET_RUNTIME_REQUEST_TIMEOUT}" \
    --node-ip=:: \
    --pod-manifest-path=/etc/kubernetes/manifests \
    --minimum-container-ttl-duration=6m0s \
    --cluster-domain=cluster.local \
    --cgroup-driver=systemd \
    --serialize-image-pulls=false \
    --v=2 \
    --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
    --pod-infra-container-image="${MACHINE_CONFIG_INFRA_IMAGE}"
[asegurap@edge-24 ~]$

staebler · 2021-04-14T21:59:42Z

pkg/types/installconfig.go

Keep this function in pkg/asset/ignition/bootstrap for now since that is the only place where it is used.

staebler · 2021-04-14T22:05:04Z

data/data/bootstrap/files/usr/local/bin/kubelet.sh.template

Would it make sense to make the name of this variable more specific to its use rather than specific to its source?

Suggested change

{{- if .APIVIPIPv6 }}

{{- if .UseIPv6ForNodeIP }}

celebdor · 2021-04-16T14:23:15Z

Applied the suggestions. Still renders right:

[asegurap@edge-24 ~]$ ssh core@fd2e:6f44:5dd8:c956::26 cat /usr/local/bin/kubelet.sh
#!/usr/bin/env bash

# shellcheck disable=SC1091  # using path on bootstrap machine
. /usr/local/bin/bootstrap-service-record.sh

/usr/bin/hyperkube \
  kubelet \
    --anonymous-auth=false \
    --container-runtime=remote \
    --container-runtime-endpoint=/var/run/crio/crio.sock \
    --runtime-request-timeout="${KUBELET_RUNTIME_REQUEST_TIMEOUT}" \
    --node-ip=:: \
    --pod-manifest-path=/etc/kubernetes/manifests \
    --minimum-container-ttl-duration=6m0s \
    --cluster-domain=cluster.local \
    --cgroup-driver=systemd \
    --serialize-image-pulls=false \
    --v=2 \
    --volume-plugin-dir=/etc/kubernetes/kubelet-plugins/volume/exec \
    --pod-infra-container-image="${MACHINE_CONFIG_INFRA_IMAGE}"
[asegurap@edge-24 ~]$

celebdor · 2021-04-16T15:10:52Z

/retest

staebler · 2021-04-16T23:55:50Z

/lgtm
/approve
/hold for e2e-metal-ipi-ovn-ipv6

openshift-ci-robot · 2021-04-16T23:56:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: staebler

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [staebler]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

osherdp · 2021-04-17T06:24:14Z

/retest

osherdp · 2021-04-18T13:29:27Z

\unhold as e2e-metal-ipi-ovn-ipv6 passes

osherdp · 2021-04-18T14:10:18Z

/unhold

openshift-bot · 2021-04-18T14:25:48Z

/retest

Please review the full test history for this PR and help us cut down flakes.

osherdp · 2021-04-18T16:32:31Z

/retest

openshift-bot · 2021-04-18T18:13:40Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-04-18T18:40:33Z

@celebdor: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-libvirt	`0c45286`	link	`/test e2e-libvirt`
ci/prow/e2e-crc	`0c45286`	link	`/test e2e-crc`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2021-04-18T18:40:54Z

@celebdor: All pull requests linked via external trackers have merged:

openshift/installer#4756

Bugzilla bug 1931974 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1931974: Prefer IPv6 hostIP on bootstrap IPv6 deployments

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2021-07-08T18:12:22Z

@sadasu: #4756 failed to apply on top of branch "release-4.7":

Applying: Prefer IPv6 hostIP on bootstrap IPv6 deployments
Using index info to reconstruct a base tree...
A	data/data/bootstrap/files/usr/local/bin/kubelet.sh
A	pkg/asset/ignition/bootstrap/common.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/asset/ignition/bootstrap/bootstrap.go
CONFLICT (content): Merge conflict in pkg/asset/ignition/bootstrap/bootstrap.go
CONFLICT (rename/delete): data/data/bootstrap/files/usr/local/bin/kubelet.sh deleted in HEAD and renamed to data/data/bootstrap/files/usr/local/bin/kubelet.sh.template in Prefer IPv6 hostIP on bootstrap IPv6 deployments. Version Prefer IPv6 hostIP on bootstrap IPv6 deployments of data/data/bootstrap/files/usr/local/bin/kubelet.sh.template left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Prefer IPv6 hostIP on bootstrap IPv6 deployments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick release-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2021-07-09T14:31:00Z

@sadasu: #4756 failed to apply on top of branch "release-4.7":

Applying: Prefer IPv6 hostIP on bootstrap IPv6 deployments
Using index info to reconstruct a base tree...
A	data/data/bootstrap/files/usr/local/bin/kubelet.sh
A	pkg/asset/ignition/bootstrap/common.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/asset/ignition/bootstrap/bootstrap.go
CONFLICT (content): Merge conflict in pkg/asset/ignition/bootstrap/bootstrap.go
CONFLICT (rename/delete): data/data/bootstrap/files/usr/local/bin/kubelet.sh deleted in HEAD and renamed to data/data/bootstrap/files/usr/local/bin/kubelet.sh.template in Prefer IPv6 hostIP on bootstrap IPv6 deployments. Version Prefer IPv6 hostIP on bootstrap IPv6 deployments of data/data/bootstrap/files/usr/local/bin/kubelet.sh.template left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Prefer IPv6 hostIP on bootstrap IPv6 deployments
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick release-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

This PR addressed issue similar to [1] for the None platform, since API VIP isn't used for None platform, the UseIPv6ForNodeIP parameter should be calculated based on the content of service CIDR. [1] openshift#4756

openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Mar 16, 2021

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Mar 16, 2021

openshift-ci-robot requested review from hardys and jhixson74 March 16, 2021 03:33

staebler reviewed Mar 16, 2021

View reviewed changes

pkg/asset/ignition/bootstrap/baremetal/template.go Outdated Show resolved Hide resolved

pkg/asset/ignition/bootstrap/baremetal/template.go Outdated Show resolved Hide resolved

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 21, 2021

staebler mentioned this pull request Mar 24, 2021

WIP Prefer IPv6 hostIP on bootstrap IPv6 deployments #4788

Closed

celebdor force-pushed the bz1931974 branch from 9699ae4 to d9b3d35 Compare April 7, 2021 01:46

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 7, 2021

celebdor force-pushed the bz1931974 branch from d9b3d35 to 7ed1923 Compare April 7, 2021 02:21

openshift-ci-robot assigned osherdp Apr 7, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 7, 2021

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 7, 2021

celebdor force-pushed the bz1931974 branch from 7ed1923 to e80022a Compare April 14, 2021 16:38

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 14, 2021

celebdor force-pushed the bz1931974 branch 2 times, most recently from 9304c8e to ba84d3a Compare April 14, 2021 17:17

staebler reviewed Apr 14, 2021

View reviewed changes

celebdor force-pushed the bz1931974 branch from ba84d3a to 0c45286 Compare April 16, 2021 13:59

openshift-ci-robot assigned staebler Apr 16, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2021

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2021

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 18, 2021

openshift-merge-robot merged commit 3cee5f9 into openshift:master Apr 18, 2021

yboaron mentioned this pull request Jun 10, 2021

Bug 1957700: None Platform - set Kubelet's node-ip param based on service net CIDR #4987

Closed

hardys mentioned this pull request Jul 7, 2021

[release-4.7] Bug 1973367: baremetal: pass IP_OPTIONS to os downloader #5013

Merged

sadasu mentioned this pull request Jul 9, 2021

Bug 1980866: [4.7] Prefer IPv6 hostIP on bootstrap IPv6 deployments #5067

Merged

hardys mentioned this pull request Apr 8, 2022

baremetal: static IP for bootstrap node #5787

Merged

celebdor deleted the bz1931974 branch November 14, 2022 14:57

danwinship mentioned this pull request Jun 28, 2024

OCPBUGS-35406: Use service CIDRs to determine bootstrap kubelet IP family #8681

Merged

Bug 1931974: Prefer IPv6 hostIP on bootstrap IPv6 deployments #4756

Bug 1931974: Prefer IPv6 hostIP on bootstrap IPv6 deployments #4756

Uh oh!

Conversation

celebdor commented Mar 16, 2021

Uh oh!

openshift-ci-robot commented Mar 16, 2021

Uh oh!

celebdor commented Mar 16, 2021

Uh oh!

Uh oh!

Uh oh!

celebdor commented Apr 7, 2021

Uh oh!

celebdor commented Apr 7, 2021

Uh oh!

celebdor commented Apr 7, 2021

Uh oh!

celebdor commented Apr 7, 2021

Uh oh!

osherdp commented Apr 7, 2021

Uh oh!

osherdp commented Apr 7, 2021

Uh oh!

celebdor commented Apr 7, 2021

Uh oh!

staebler commented Apr 7, 2021

Uh oh!

staebler commented Apr 7, 2021

Uh oh!

celebdor commented Apr 14, 2021

Uh oh!

staebler Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

staebler Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

celebdor commented Apr 16, 2021

Uh oh!

celebdor commented Apr 16, 2021

Uh oh!

staebler commented Apr 16, 2021

Uh oh!

openshift-ci-robot commented Apr 16, 2021

Uh oh!

osherdp commented Apr 17, 2021

Uh oh!

osherdp commented Apr 18, 2021

Uh oh!

osherdp commented Apr 18, 2021

Uh oh!

openshift-bot commented Apr 18, 2021

Uh oh!

osherdp commented Apr 18, 2021

Uh oh!

openshift-bot commented Apr 18, 2021

Uh oh!

openshift-ci bot commented Apr 18, 2021

Uh oh!

openshift-ci-robot commented Apr 18, 2021

Uh oh!

openshift-cherrypick-robot commented Jul 8, 2021

Uh oh!

openshift-cherrypick-robot commented Jul 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants