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

Bug 1823967: Add the --pod-infra-container-image flag to the kubelet service #3712

Merged
merged 1 commit into from Jun 9, 2020

Conversation

umohnani8
Copy link
Contributor

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1823967

Override the --pod-infra-container-image flag to point to the pause
image from the release payload. By default, this flag is set to k8s.gcr.io/pause:3.1
and the kubelet asks the runtime for the image status of this image every few minutes,
which is why we were seeing the warning logs in cri-o saying that this image was not
found. This is avoided by making the pod-infra-container-image flag point to the actual
pause image being used from the release payload.

The MCO PR is openshift/machine-config-operator#1776

Signed-off-by: Urvashi Mohnani umohnani@redhat.com

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jun 3, 2020
@openshift-ci-robot
Copy link
Contributor

@umohnani8: This pull request references Bugzilla bug 1823967, which is valid. 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.6.0) matches configured target release for branch (4.6.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1823967: Add the --pod-infra-container-image flag to the kubelet service

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.

@umohnani8
Copy link
Contributor Author

@abhinavdahiya @rphillips PTAL
What do I need to run to autogenerate the assets_vfsdata.go file?

@abhinavdahiya
Copy link
Contributor

hmm. I don't think it is fair for all users of kubelet to set this value :( is there no way kubelet can offload this responsibility to container runtime?

What do I need to run to autogenerate the assets_vfsdata.go file?
no action is required, we generate it while building the binary.

@umohnani8
Copy link
Contributor Author

@abhinavdahiya not really. The runtime overrides this value when you set it there, but the kubelet needs to to know about the pause image being used, especially since it changes hash references, so that it can get the images and accurately gc the old ones.

@umohnani8
Copy link
Contributor Author

/retest

1 similar comment
@umohnani8
Copy link
Contributor Author

/retest

@umohnani8
Copy link
Contributor Author

/retest

@rphillips
Copy link
Contributor

lgtm

@umohnani8
Copy link
Contributor Author

@abhinavdahiya @patrickdillon this is ready, PTAL

@patrickdillon
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2020
# Need to set the --pod-infra-container-image flag for the kubelet to point to the pause image from the payload
# So we add MACHINE_CONFIG_INFRA_IMAGE to an environment file and source that in the kubelet service

. /usr/local/bin/release-image.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

For this to correctly load/finish requires that release image has been downloaded by the release-image.service unit.

How will we ensure that kubelet waits or keeps retrying until I that unit suceeds.
If there is a failure will kubelet service retry or just fail and sit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kubelet can't start until crio has been started. And https://github.com/openshift/installer/blob/master/data/data/bootstrap/systemd/units/crio-configure.service.template#L5 ensures that the release image has been downloaded before crio is started.
But to make sure, I can add After/Wants=release-image.service to the kubelet service here https://github.com/openshift/installer/pull/3712/files#diff-367cd6ecdc6c4f5bbe9560468fad15f3R4 as well to ensure you can't start kubelet till release-image.service has been successful. Wdyt?

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 added release-image.service to After and Wants in the kubelet service template below.

Override the --pod-infra-container-image flag to point to the pause
image from the release payload. By default, this flag is set to k8s.gcr.io/pause:3.1
and the kubelet asks the runtime for the image status of this image every few minutes,
which is why we were seeing the warning logs in cri-o saying that this image was not
found. This is avoided by making the pod-infra-container-image flag point to the actual
pause image being used from the release payload.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2020
@abhinavdahiya
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2020
@nee1esh
Copy link

nee1esh commented Jun 8, 2020

/retest

@patrickdillon
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2020
@openshift-bot
Copy link
Contributor

/retest

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-ci-robot
Copy link
Contributor

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

Test name Commit Details Rerun command
ci/prow/e2e-openstack d9d1f3d link /test e2e-openstack
ci/prow/e2e-ovirt d9d1f3d link /test e2e-ovirt

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-merge-robot openshift-merge-robot merged commit 145e5cc into openshift:master Jun 9, 2020
@openshift-ci-robot
Copy link
Contributor

@umohnani8: All pull requests linked via external trackers have merged: openshift/machine-config-operator#1776, openshift/installer#3712. Bugzilla bug 1823967 has been moved to the MODIFIED state.

In response to this:

Bug 1823967: Add the --pod-infra-container-image flag to the kubelet service

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.

@umohnani8
Copy link
Contributor Author

/cherry-pick release-4.5

@openshift-cherrypick-robot

@umohnani8: new pull request created: #3731

In response to this:

/cherry-pick release-4.5

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants