-
Notifications
You must be signed in to change notification settings - Fork 22
Conversation
vagrant/_commons.sh
Outdated
;; | ||
esac | ||
#sudo git clone https://github.com/kubernetes-sigs/kubespray $kubespray_folder | ||
sudo git clone https://github.com/electrocucaracha/kubespray $kubespray_folder |
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.
my expectation is when running on Clear Linux we use k8s what Clear linux ships in cloud-native-basic
bundle. The bundle provides easy-to-use installation scripts in /usr/share/clr-k8s-examples
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.
AFAIK this PR does that https://github.com/kubernetes-sigs/kubespray/pull/3855/files
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.
Does it work for you? A quick look tells me it installs containers-basic
(and not cloud-native-basic
)
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 did some checking and apparently kubespray always uses the upstream binaries and not what the OS provides (for cri-o, kubeadm/kubelet). It also does not give you kata-containers.
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.
We can do those changes after Kubernetes is deployed. Eventually we can propose those changes to the Kubespray community
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.
We can do those changes after Kubernetes is deployed.
I don't follow what changes? My proposal is for Clear install we use cloud-native-basic
and the setup scripts it provides.
vagrant/postchecks_qat_driver.sh
Outdated
@@ -16,10 +16,10 @@ echo "QAT driver validation" | |||
|
|||
# Driver validation | |||
supported_dev="c6xx dh895xcc" | |||
qat_svc=$(sudo service qat_service status | grep "There is .* QAT acceleration device(s) in the system:") | |||
qat_svc=$(sudo systemctl status qat_service | grep "There is .* QAT acceleration device(s) in the system:") |
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.
It seems that adf_ctl status
return value is zero if acceleration devices are present in the system. If you can verify this, there is no need to parse the (fragile) service logging output.
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.
It returns zero if the command was executed properly:
[vagrant@localhost ~]$ sudo /usr/local/bin/adf_ctl status
Checking status of all devices.
There is 0 QAT acceleration device(s) in the system:
[vagrant@localhost ~]$ echo $?
0
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.
Did you try the negative case too? If both work then this might be a cleaner way to implement the check.
10bcd1a
to
e0fe4b5
Compare
adf7ae2
to
72e5f98
Compare
92bfa68
to
6e0d415
Compare
I see you force pushed the branch a few times. Please comment if the PR is ready for another review round or if there are still unresolved issues left. |
@ipuustin it's failing when Kata is selected as container runtime. I'll let you know when this can be ready to be reviewed |
6e0d415
to
2682f39
Compare
I don't think I have any more comments. @electrocucaracha can you try out squashing some of the commits? |
e52cecb
to
53cf1e7
Compare
@mythi I created two branches in my repo, the backup contains the original version with 43 commits ahead of intel:master and the new refactored clearlinux branch with 32 commits ahead. I'm finalizing the tests and I'll let you know when I'm ready. |
@electrocucaracha thanks! |
@electrocucaracha I checked the latest changes and I'm still not happy with the commits. There are several "intermediate stages" still:
For future PRs, we also need to pay more attention to commit messages. "fix something" is not good unless the message also explains what was broken. |
This change adds support to ClearLinux distribution using a Virtual Machine. It requires testing on Bare-Metal server.
53cf1e7
to
ccf5714
Compare
This test.sh bash script tests multiple scenarios (different linux distros, container managers, container runtimes) using different screen sessions.
ccf5714
to
c0f02be
Compare
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.
@electrocucaracha nice!
This change is an effort to add support of ClearLinux Distribution.