Enable experimental modules when rpm-ostree version >= 2021.9 #8202

zhengtianbao · 2021-11-17T07:05:01Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

rpm-ostree provides experimental support for modules since version 2021.9, so we should use command rpm-ostree ex module enable to enable the cri-o stream first and then install from that stream.

refer: https://discussion.fedoraproject.org/t/cri-o-no-longer-available-through-rpm-ostree/33908

Which issue(s) this PR fixes:

Fixes #8197

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

[cri-o] Enable experimental modules when rpm-ostree version >= 2021.9

k8s-ci-robot · 2021-11-17T07:05:09Z

Hi @zhengtianbao. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

k8s-triage-robot · 2021-11-17T09:11:03Z

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla

hardoverflow · 2021-11-17T11:23:30Z

I have checked out the #8202 and can confirm that it works.

/lgtm

k8s-ci-robot · 2021-11-17T11:23:41Z

@hardoverflow: changing LGTM is restricted to collaborators

In response to this:

I have checked out the #8202 and can confirm that it works.

/lgtm

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.

cristicalin

Interesting idea for sure but I'm skeptical about relying on experimental features in tools (ex stands for experimental in rpm-ostree).

cristicalin · 2021-11-17T20:38:19Z

roles/container-engine/cri-o/tasks/main.yaml

@@ -11,6 +11,21 @@
  set_fact:
    is_ostree: "{{ ostree.stat.exists }}"

+- name: get ostree version
+  shell: "set -o pipefail && rpm-ostree --version |grep 'Version:' |awk '{print $2}'| sed \"s/'//g\" "


Why so complex?

Suggested change

shell: "set -o pipefail && rpm-ostree --version |grep 'Version:' |awk '{print $2}'| sed \"s/'//g\" "

shell: "rpm-ostree --version | awk -F\' '/Version/{print $2}'"

Thanks for your advice.

cristicalin · 2021-11-17T20:38:36Z

roles/container-engine/cri-o/tasks/main.yaml

+- name: get ostree version
+  shell: "set -o pipefail && rpm-ostree --version |grep 'Version:' |awk '{print $2}'| sed \"s/'//g\" "
+  args:
+    executable: /bin/bash


No need for this.

for ansible-lint.

cristicalin · 2021-11-17T20:39:39Z

roles/container-engine/cri-o/tasks/main.yaml

+    ostree_version: "{{ ostree_version_on_server.stdout }}"
+  when:
+    - is_ostree
+    - 'ostree_version_on_server.stdout is defined'


The code would be cleaner if you just checked ostree_version_on_server is succceded (also not a fan of this extremely descriptive name) in case rpm-ostree is not installed and allowed the previous task to ignore the failure.

Yeah, it's smells not good, thanks again.

cristicalin · 2021-11-17T20:40:28Z

roles/container-engine/cri-o/tasks/crio_repo.yml

+  become: true
+  when:
+    - is_ostree
+    - ostree_version is version('2021.9', '>=')


Unless ostree_version is in detaults/main.yml this would fail.

That's because I assume if is_ostree is defined then ostree_version will be defined too.

roles/container-engine/cri-o/tasks/main.yaml

cristicalin · 2021-11-18T08:53:11Z

General question, do we have any way to test this in CI?

cristicalin · 2021-11-18T09:23:51Z

/ok-to-test

oomichi · 2021-11-19T22:07:07Z

/lgtm

k8s-ci-robot · 2021-11-22T10:25:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, zhengtianbao

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:

~~OWNERS~~ [floryut]

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

…etes-sigs#8202) * Enable experimental modules when rpm-ostree version >= 2021.9 * cleanup code

Enable experimental modules when rpm-ostree version >= 2021.9

0d8ec0f

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 17, 2021

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 17, 2021

k8s-ci-robot requested review from EppO and jayonlau November 17, 2021 07:05

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 17, 2021

cristicalin reviewed Nov 17, 2021

View reviewed changes

cleanup code

722782c

cristicalin reviewed Nov 18, 2021

View reviewed changes

roles/container-engine/cri-o/tasks/main.yaml Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 18, 2021

k8s-ci-robot assigned oomichi Nov 19, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 19, 2021

floryut added kind/container-managers Containers section in the release note and removed kind/bug Categorizes issue or PR as related to a bug. labels Nov 22, 2021

floryut approved these changes Nov 22, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 22, 2021

k8s-ci-robot merged commit a6fcf2e into kubernetes-sigs:master Nov 22, 2021

floryut mentioned this pull request Dec 21, 2021

Release Proposal v2.18 #8325

Closed

sakuraiyuta pushed a commit to sakuraiyuta/kubespray that referenced this pull request Apr 16, 2022

Enable experimental modules when rpm-ostree version >= 2021.9 (kubern…

45f1923

…etes-sigs#8202) * Enable experimental modules when rpm-ostree version >= 2021.9 * cleanup code

LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Jun 28, 2023

Enable experimental modules when rpm-ostree version >= 2021.9 (kubern…

98299a1

…etes-sigs#8202) * Enable experimental modules when rpm-ostree version >= 2021.9 * cleanup code

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable experimental modules when rpm-ostree version >= 2021.9 #8202

Enable experimental modules when rpm-ostree version >= 2021.9 #8202

zhengtianbao commented Nov 17, 2021 •

edited by floryut

Loading

k8s-ci-robot commented Nov 17, 2021

k8s-triage-robot commented Nov 17, 2021

hardoverflow commented Nov 17, 2021

k8s-ci-robot commented Nov 17, 2021

cristicalin left a comment

cristicalin Nov 17, 2021

zhengtianbao Nov 18, 2021

cristicalin Nov 17, 2021

zhengtianbao Nov 18, 2021

cristicalin Nov 17, 2021

zhengtianbao Nov 18, 2021

cristicalin Nov 17, 2021

zhengtianbao Nov 18, 2021

cristicalin commented Nov 18, 2021

cristicalin commented Nov 18, 2021

oomichi commented Nov 19, 2021

k8s-ci-robot commented Nov 22, 2021

	shell: "set -o pipefail && rpm-ostree --version \|grep 'Version:' \|awk '{print $2}'\| sed \"s/'//g\" "
	shell: "rpm-ostree --version \| awk -F\' '/Version/{print $2}'"

Enable experimental modules when rpm-ostree version >= 2021.9 #8202

Enable experimental modules when rpm-ostree version >= 2021.9 #8202

Conversation

zhengtianbao commented Nov 17, 2021 • edited by floryut Loading

k8s-ci-robot commented Nov 17, 2021

k8s-triage-robot commented Nov 17, 2021

hardoverflow commented Nov 17, 2021

k8s-ci-robot commented Nov 17, 2021

cristicalin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristicalin commented Nov 18, 2021

cristicalin commented Nov 18, 2021

oomichi commented Nov 19, 2021

k8s-ci-robot commented Nov 22, 2021

zhengtianbao commented Nov 17, 2021 •

edited by floryut

Loading