tests/ansible: Add fedora linux distros #318
Conversation
|
/test |
|
Looks good although doesn't seem sufficient. I'm getting: failure on Fedora 39. Let me take a look what needs to be added... |
There was a problem hiding this comment.
@tylerfanelli could you please additionally address the places where ansible_distribution == CentOS is used and add another branch to install docker via https://download.docker.com/linux/fedora/docker-ce.repo repo so this actually becomes usable?
Note: there is also the karmab/kcli@16d424a issue that needs to be configured. I gave it a try (only added the cgroup fix) but still it wasn't really successful (F39), I guess it might require some Have you succeeded in your attempts? diff --git a/tests/e2e/ansible/group_vars/all b/tests/e2e/ansible/group_vars/all
index 43f145d..a81975a 100644
--- a/tests/e2e/ansible/group_vars/all
+++ b/tests/e2e/ansible/group_vars/all
@@ -6,6 +6,9 @@ build_pkgs:
- make
- gcc
- qemu-user-static
+ fedora:
+ - make
+ - gcc
centos:
- make
- gcc
@@ -16,6 +19,9 @@ kubeadm_pkgs:
ubuntu:
- conntrack
- socat
+ fedora:
+ - conntrack
+ - socat
centos:
- conntrack
- socat
@@ -23,6 +29,8 @@ k8s_version: v1.24.0
test_pkgs:
ubuntu:
- jq
+ fedora:
+ - jq
centos:
- jq
target_arch: "{{ 'amd64' if ansible_architecture == 'x86_64' else ansible_architecture }}"
diff --git a/tests/e2e/ansible/install_build_deps.yml b/tests/e2e/ansible/install_build_deps.yml
index e1621db..691ae8b 100644
--- a/tests/e2e/ansible/install_build_deps.yml
+++ b/tests/e2e/ansible/install_build_deps.yml
@@ -38,7 +38,7 @@
- name: Install qemu-user-static
shell: docker run --rm --privileged multiarch/qemu-user-static:7.2.0-1 --reset -p yes
when: qemu_user_static_exist.rc != 0
- when: ansible_distribution == "CentOS"
+ when: ansible_distribution in ["CentOS", "Fedora"]
# Undo the installation.
#
- name: Uninstall build dependencies
diff --git a/tests/e2e/ansible/install_containerd.yml b/tests/e2e/ansible/install_containerd.yml
index 4f6d102..cb6cab4 100644
--- a/tests/e2e/ansible/install_containerd.yml
+++ b/tests/e2e/ansible/install_containerd.yml
@@ -31,6 +31,10 @@
containerd config default > /etc/containerd/config.toml
args:
executable: /bin/bash
+ - name: Enable SystemdCgroups
+ shell: |
+ sed -i 's/SystemdCgroup = .*/SystemdCgroup = true/' /etc/containerd/config.toml
+ when: ansible_distribution == "Fedora"
- name: Restart containerd service
service:
name: containerd
diff --git a/tests/e2e/ansible/install_docker.yml b/tests/e2e/ansible/install_docker.yml
index 9e3572d..7204e95 100644
--- a/tests/e2e/ansible/install_docker.yml
+++ b/tests/e2e/ansible/install_docker.yml
@@ -59,6 +59,24 @@
name: docker
state: present
when: docker_exist.rc != 0 and ansible_distribution == "Ubuntu" and ansible_distribution_version == "22.04"
+- name: Handle docker installation on CentOS.
+ block:
+ - name: Install yum-utils
+ dnf:
+ name: yum-utils
+ state: present
+ - name: Add docker yum repo
+ shell: yum-config-manager --add-repo https://download.docker.com/linux/fedora/docker-ce.repo
+ args:
+ creates: /etc/yum.repos.d/docker-ce.repo
+ - name: Install docker packages
+ dnf:
+ name:
+ - containerd.io
+ - docker-ce
+ - docker-ce-cli
+ state: present
+ when: docker_exist.rc != 0 and ansible_distribution == "Fedora"
- name: Handle docker installation on CentOS.
block:
- name: Install yum-utils |
I noticed that one (after initial attempts) but the zram removal did not really help. So the questions stays, @tylerfanelli have you succeeded in running the |
|
@ldoktor zram removal is what worked for me. Besides the changes I made in the PR and following the Vagrantfile CentOS additions, I'm not too sure what else i did different. This is for F39. |
|
I added the following changes on top of this PR (including removal of zram-generator-defaults) and could progress on a F39 VM. Changes Error |
|
Now I hit this error after running on a F39 VM with 4vCPUs @BbolroC @stevenhorsman any thoughts ? |
Hi, @bpradipt Thanks for the notice. I've reproduced the error on my F39. Due to the clue It seems that I got the build successfully completed again when I ran the following: I think there should be a configuration step for operator/tests/e2e/ansible/install_build_deps.yml Lines 32 to 41 in e290f0f |
|
Oh, sorry. I had not gone over the comments for the PR when I wrote this. You guys have already mentioned an issue around FYI: what made me struggle to run a cluster on my F39 while debugging the It seems that SwapOnZRAM is enabled by default on F39. I was able to start This would not be the case for others, but I just wanted to share this. Thanks! |
|
I have managed to finish the test successfully after configuring the following manually:
|
|
Thanks a lot @BbolroC . It might make sense to add the zram-generator-defaults removal and qemu-user-static installation as part of the ansible playbook itself. I'll do a rerun on my setup with your instructions and update |
|
I could run successfully as well with the following changes on top of this PR |
|
@tylerfanelli fyi |
Yep, @bpradipt version works well for me as well, although I wouldn't recommend restricting the versions to 39. |
I'll preferably stick to a specific version for repeatability. |
I just verified it works with F38 as well and I don't see any reason for it not to work on 40+ |
tylerfanelli
force-pushed
the
run-local-fedora
branch
from
72a09b2
to
49d2d00
Compare
|
@bpradipt I've added your changes, and added a |
|
@ldoktor Can you re-test this PR? |
tylerfanelli
force-pushed
the
run-local-fedora
branch
2 times, most recently
from
0165b17
to
3cf3b65
Compare
|
@tylerfanelli indentation error: |
tylerfanelli
force-pushed
the
run-local-fedora
branch
2 times, most recently
from
04f7d60
to
03b44cb
Compare
|
@wainersm Fixed, thanks. |
There was a problem hiding this comment.
Works well for me although I'd really like to remove the distro-version check. It works on F38 and F40 will be released soon. We don't have such checks for ubuntu versions and everything works on several versions while with the checks it simply only works on F39.
|
I tested it in a Fedora 39 VM, it works. How I did setup the VM:
|
I agree with @ldoktor |
|
FYI: Rebasing onto main is necessary because a structure of the workflow for e2e tests has been changed. |
@ldoktor @wainersm Even for Ubuntu there are explicit version checks - https://github.com/confidential-containers/operator/blob/main/tests/e2e/ansible/install_docker.yml#L61 We rely on external packages for setting up the environment. Docker might decide not to ship packages for the latest Fedora version. Likewise, kubeadm packages might need different handling for different distro versions or might not be available for a specific distro version, resulting in a bad first-time experience. We have seen this cycle repeat numerous times. Pinned versions (to the extent practical and feasible) help with repeatability. Additionally, we might not be able to keep testing and updating for every distro version. In future, if it makes sense to drop older distro versions and stick to new ones, we can make the change and suggest every dev to use the recommended distro version. My understanding is that Anyway, if you still prefer supporting generic Fedora, I would request @tylerfanelli to make the necessary changes and let's merge this PR. May be additionally update the readme with a note on which Fedora version it's tested to reduce the probability of a bad initial dev experience. |
I understand your point but if I am to chose between |
For dev and CI environments my preference is to use tools with a fixed scope with repeatability since the primary intent is to not fix environment issues. Separation of concerns :-) Anyways, since you prefer a generic approach I'm ok with it. @tylerfanelli would you be able to work on the requested changes by @ldoktor and remove the distro version checks ? |
I understand, Pradipta, my preference is to do best-approach way and pin the CI versions, which should serve as a recommendation to developers on which env is supported while allowing some flexibility to use whatever they might want as long as they can adjust the scripts, which is actually the case here. We do not use Fedora in the CI at all, it's just a nice-to-have addition. |
tylerfanelli
force-pushed
the
run-local-fedora
branch
from
03b44cb
to
7e8dd91
Compare
Fedora also can be used to run k8s clusters locally. Add the fedora requirements to allow for testing. Tested on Fedora 39. Co-authored-by: Pradipta Banerjee <pradipta.banerjee@gmail.com> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
tylerfanelli
force-pushed
the
run-local-fedora
branch
from
7e8dd91
to
55787f5
Compare
Fedora also can be used to run k8s clusters locally. Add the fedora requirements to allow for testing.