-
Notifications
You must be signed in to change notification settings - Fork 45
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
Upgrade containerd #2369
Upgrade containerd #2369
Conversation
Hello nicolast,My role is to assist you with the merge of this Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
8fa2264
to
05ea261
Compare
For reviewers, the most important changes are
|
05ea261
to
f1df9b5
Compare
@bert-e approve |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
The following options are set: approve |
Moved into 2.6.0. Needs field testing, too big a change for a .1. |
…t/upgrade-containerd * origin/development/2.6: images: enrich `metalk8s-utils` image deps: Pin pytest version deps: Bump PyYAML to 5.3.1 in buildchain deps: Regenerate compiled requirements eve: Add `text` in JUnit containing artifact Url eve: Add `text` support for JUnit generation eve: Add `message` support for JUnit generation Conflicts: CHANGELOG.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the most part! Some suggested changes on tests, maybe we want to move them in a debt ticket, as you wish.
def given_check_pod_status(request, host, k8s_client, label, expected_status): | ||
ssh_config = request.config.getoption('--ssh-config') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's not related to your changes, but there's a ssh_config
fixture to simplify this a bit:
def given_check_pod_status(request, host, k8s_client, label, expected_status): | |
ssh_config = request.config.getoption('--ssh-config') | |
def given_check_pod_status(ssh_config, host, k8s_client, label, expected_status): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
@pytest.fixture | ||
def utils_pod(k8s_client, utils_image): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to create another fixture, @when
already makes the create_utils_pod
a fixture. This would allow us to make use of a BDD parser:
_UTILS_POD_PARSER = parsers.parse(
"we create a utils Pod with labels {labels:JSON} "
"and annotations {annotations:JSON}",
extra_types=dict(JSON=json.loads),
)
@when(_UTILS_POD_PARSER)
def utils_pod(k8s_client, utils_image, labels, annotations):
# create the Pod with right labels and annotations, which are
# passed in as Python dicts already
try:
yield pod_name
finally:
# clean-up of the Pod
@then("I can run '{command}' in the utils Pod")
def run_in_utils_pod(utils_pod, command):
# If we had such a helper, we could do:
kubectl_exec(pod_name=utils_pod, command=command)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, that's what I tried at first (how could one perform resource cleanup otherwise?). However, when executing the test using that implementation, nothing happened. My when
thing, which is then a generator, would not be next
ed, so no code executed, no Pod created,... (to my surprise). Skimming the docs back then, there's no reference of when
s being permitted to be a generator.
I'll try again with a super-simple test...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on Slack, one can't use a generator in a @when
implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, but it seems we can in a @given
(and in your context, creating a utils Pod could make sense as a "Given" step).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, no :) I am not actually using the utils
Pod, its merely a canary (hence, not executing any commands in it). What this test wants to validate is that any Pod with some specific set of annotations will be fully functional at some point (i.e., our CRI supports the seccomp
profile annotations). The Pod is not an input to the test, its creation and validation is the test.
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye nicolast. |
Upgrade
containerd
to 1.2.13, among others to include a fix for containerd/containerd#3125. Also, enableseccomp
support to handle #2259.This implies we no longer ship the
containerd
package as found in EPEL, but instead build one as part of the MetalK8s build and ship it in themetalk8s-scality
repository.