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

Adding s390x arch support in k8s 1.28 provider #1201

Closed

Conversation

chandramerla
Copy link
Contributor

What this PR does / why we need it:
This PR enables kubernetes 1.28 provider to be s390x arch compatible in SLIM(=true) mode. This is one of the initial steps to enabling Kubevirt CI for s390x

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:
With these changes I've run all the steps in KUBEVIRTCI_LOCAL_TESTING.md guide to test provider in both x86 and s390x machines, with specific mention of some details as below:

  1. Have not tested with KSM and SWAP ON and keeping SLIM=true for s390x, as this initial PR is targeting to add support of s390x for 1.28 provider in slim mode.
  2. Tested make cluster-up on s390x with even in single stack mode
  3. Have tested make cluster-sync on x86 (with KUBEVIRT_CDI_DEPLOY=false as otherwise there is an error 'error when creating "..../uploadproxy-nodeport.yaml": namespaces "cdi" not found')
  4. make functests on x86 are failing locally with 'timer expired' errors, etc., after I resolve the issue mentioned following the workaround

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Release note:

NONE

* Used qemu-monitor command to add secondary network interface to avoid the primary network interface having names other than eth0. As  based on the MAC address of eth0 interface we are fetching the IP of the VM from DHCP server, it needs to be MAC of the primary network interface, to obtain the IP for the interface.
* Bumped calico CNI version from 3.18.0 to 3.27.2 (latest currently), as 3.18.0 doesn't support s390x arch.

Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 11, 2024
@kubevirt-bot
Copy link
Contributor

Hi @chandramerla. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

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-sigs/prow repository.

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign qinqon for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@chandramerla
Copy link
Contributor Author

/test ?

@kubevirt-bot
Copy link
Contributor

@chandramerla: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test ?

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-sigs/prow repository.

@chandramerla
Copy link
Contributor Author

chandramerla commented Jun 11, 2024

/cc @brianmcarey

@chandramerla
Copy link
Contributor Author

/unassign @brianmcarey

@chandramerla
Copy link
Contributor Author

/cc @brianmcarey

@chandramerla
Copy link
Contributor Author

chandramerla commented Jun 11, 2024

@ Reviewers: Happy to take you through the changes in a call if that helps. Please let me know. Thanks!

@brianmcarey
Copy link
Member

/test check-provision-k8s-1.28

@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 25, 2024
Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Jun 25, 2024
This reverts commit 3cdfc8f.

Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
@brianmcarey
Copy link
Member

/test check-provision-k8s-1.28

@brianmcarey
Copy link
Member

/test ?

@kubevirt-bot
Copy link
Contributor

@brianmcarey: The following commands are available to trigger required jobs:

  • /test check-provision-alpine-with-test-tooling
  • /test check-provision-centos-base
  • /test check-provision-k8s-1.28
  • /test check-provision-k8s-1.29
  • /test check-provision-k8s-1.30
  • /test check-provision-manager
  • /test check-up-kind-ovn
  • /test check-up-kind-sriov

The following commands are available to trigger optional jobs:

  • /test check-gocli
  • /test check-up-kind-1.27-vgpu

Use /test all to run the following jobs that were automatically triggered:

  • check-provision-alpine-with-test-tooling
  • check-provision-k8s-1.28
  • check-provision-k8s-1.29
  • check-provision-k8s-1.30
  • check-provision-manager
  • check-up-kind-1.27-vgpu
  • check-up-kind-sriov

In response to this:

/test ?

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-sigs/prow repository.

@brianmcarey
Copy link
Member

/test check-gocli
/test check-provision-centos-base

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2024
…90x are broken due to various issues

Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 18, 2024
@chandramerla chandramerla marked this pull request as ready for review July 19, 2024 04:54
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 19, 2024
…arg to docker build

Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
Copy link
Member

@brianmcarey brianmcarey left a comment

Choose a reason for hiding this comment

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

Thanks for this @chandramerla

My main worry with this is that the k8s-1.28 provider is soon to be removed as we have started adding the k8s-1.31.

It may be better to target the k8s-1.30 provider.

@@ -48,7 +48,7 @@ export KUBEVIRTCI_GOCLI_CONTAINER=quay.io/kubevirtci/gocli:latest
### start cluster

```bash
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.28
Copy link
Member

Choose a reason for hiding this comment

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

Why are you dropping the version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey I just made the version consistent across this file :)
I saw in a place it is 1.27

(cd cluster-provision/k8s/1.28; ../provision.sh)
In other two places as 1.30
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.30
In other two places 1.21
export PHASES=k8s; (cd cluster-provision/k8s/1.21; ../provision.sh)

So everywhere I changed to 1.28

Let me change all these to 1.30

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in k8s-1.30-provider-slim-s390x branch

Run the `k8s` step as much as needed. It reuses the intermediate image that was created
by the `linux` phase.
Note :
1. By default when you run `k8s` phase alone, it uses centos9 image specified in cluster-provision/k8s/base-image, not the one built locally in the `linux` phase. So, to make `k8s` phase use the locally built centos9 image, update cluster-provision/k8s/base-image with the locally built image name and tag (default: quay.io/kubevirtci/centos9:latest)
Copy link
Member

Choose a reason for hiding this comment

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

thanks for adding this

curl -L -o dockerize-linux-$BUILDARCH.tar.gz https://github.com/jwilder/dockerize/releases/download/$DOCKERIZE_VERSION/dockerize-linux-amd64-$DOCKERIZE_VERSION.tar.gz; \
else \
# Temporary till s390x support is upstreamed to dockerize (https://github.com/jwilder/dockerize/pull/200)
curl -L -o dockerize-linux-$BUILDARCH.tar.gz https://github.com/ibm-jitendra/kubevirt_pkgs/raw/main/dockerize-linux-s390x.tar.gz; \
Copy link
Member

Choose a reason for hiding this comment

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

Is there any timeline for the upstreaming of this? I would prefer not to rely on a random tarball.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey : No update as of now. We are tried to reach upstream owner on PR, but no luck. Now, Vamsi is trying to reach him on LinkedIn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for @vamsikrishna-siddu for the followup. Now changes are upstreamed.
This is addressed in k8s-1.30-provider-slim-s390x branch

@@ -0,0 +1 @@
root=/dev/vda1 ro no_timer_check console=tty0 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to have a separate commit for these kernel args that explains why some of the different options are required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were the defaults within the generic cloud image of centos9 for s390x. I've copied them to this file to mainly keep parity with x86 based kernel args, which I think exists for configuring args for kernel.

Will try to come up with a readme around this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this as separate commit in k8s-1.30-provider-slim-s390x branch, where in commit I've explained all the details.
6f3f133

-device AC97 \
-uuid $(cat /proc/sys/kernel/random/uuid) \
${QEMU_ARGS}"
else
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that the else would cover the more common x86_64 case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand you right, have s390x case in if block and x86_64 case in else block. I will change it accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is addressed in k8s-1.30-provider-slim-s390x branch

@@ -252,21 +255,21 @@ func provisionCluster(cmd *cobra.Command, args []string) (retErr error) {
if err != nil {
return err
}
err = _cmd(cli, nodeContainer(prefix, nodeName), "if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images vagrant@192.168.66.101:/tmp/extra-pre-pull-images; fi", "copying /scripts/extra-pre-pull-images if existing")
err = _cmd(cli, nodeContainer(prefix, nodeName), fmt.Sprintf("if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images %s@192.168.66.101:/tmp/extra-pre-pull-images; fi", sshUser), "copying /scripts/extra-pre-pull-images if existing")
Copy link
Member

Choose a reason for hiding this comment

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

I think some of this ssh handling has changed since #1209 was merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey This PR changes are already part of my branch. But will check if I need to adjust it. As PR#1209 is quite big digesting it still.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't require to change this as in #1029 these function calls have not changed.

@@ -301,6 +315,11 @@ kubeadm_raw_ipv6=/tmp/kubeadm_ipv6.conf
kubeadm_manifest="/etc/kubernetes/kubeadm.conf"
kubeadm_manifest_ipv6="/etc/kubernetes/kubeadm_ipv6.conf"

# envsubst pkg is not available by default in s390x Architecture.
if [ "$arch" == "s390x" ]; then
dnf install -y gettext
Copy link
Member

Choose a reason for hiding this comment

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

We could probably include this when we install this with other rpm dependencies in provision.sh

I don't think we need an if statement here - if its already installed on x86 it won't do anything anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in k8s-1.30-provider-slim-s390x branch, by moving the line dnf install -y gettext to the end of provision.sh

@@ -14,13 +16,13 @@ export ISTIO_BIN_DIR="/opt/istio-${ISTIO_VERSION}/bin"
EOF
source $KUBEVIRTCI_SHARED_DIR/shared_vars.sh

# Install modules of the initrd kernel
# Install modules of the initrd kernel. These modules extend the kernel's functionality, providing support for various hardware devices, file systems, network protocols, KVM/virtualization, and other features.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we need this additional comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove it. Added it as initially for my reference to understand the sequence of steps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in k8s-1.30-provider-slim-s390x branch

dnf install -y "kernel-modules-$(uname -r)"

# Resize root partition
dnf install -y cloud-utils-growpart
if growpart /dev/vda 1; then
resize2fs /dev/vda1
if growpart /dev/vda 1; then #growpart adjusts the partition size to fill the available space on the disk
Copy link
Member

Choose a reason for hiding this comment

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

same here - not sure if these additional comments are needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in k8s-1.30-provider-slim-s390x branch

dnf install -y openvswitch2.16
#openvswitch for s390x is not available from the centos default repos.
if [ "$ARCH" == "s390x" ]; then
dnf install -y https://kojipkgs.fedoraproject.org//packages/openvswitch/2.16.0/2.fc36/s390x/openvswitch-2.16.0-2.fc36.s390x.rpm
Copy link
Member

Choose a reason for hiding this comment

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

is this the best available source for openvswitch for s390x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, unfortunately.
I checked today also, couldn't find it else where.

Copy link
Contributor Author

@chandramerla chandramerla left a comment

Choose a reason for hiding this comment

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

@brianmcarey Thanks for your review comments. I've added my initial replies. I will work on them as in my replies.

Also as we discussed earlier, I've added all changes required for publishing multi-arch gocli, centos9 and k8s provider images at here and at kubevirt/project-infra#3566. I've kept multi-arch publish related changes required in kubevirtci repo separately here, so that it is easier for review. But if you prefer merging it to this #1201 PR for review, please let me know. I'll be awaiting for your feedback on this.

On targeting 1.30 provider for enable for s390x, I will create a separate branch and start doing changes there. The issue that stopped me earlier to start with newer providers for s390x support for k8s equivalent crio version builds aren't available or broken for s390x. This time if I face such issues, I'm thinking to try with containerd in place of crio just to unblock.

@@ -48,7 +48,7 @@ export KUBEVIRTCI_GOCLI_CONTAINER=quay.io/kubevirtci/gocli:latest
### start cluster

```bash
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey I just made the version consistent across this file :)
I saw in a place it is 1.27

(cd cluster-provision/k8s/1.28; ../provision.sh)
In other two places as 1.30
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.30
In other two places 1.21
export PHASES=k8s; (cd cluster-provision/k8s/1.21; ../provision.sh)

So everywhere I changed to 1.28

Let me change all these to 1.30

@@ -59,7 +59,7 @@ make cluster-up
#### start cluster with prometheus, alertmanager and grafana
To enable prometheus, please also export the following variables before running `make cluster-up`:
```bash
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will change

curl -L -o /initramfs-amd64.img http://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/images/pxeboot/initrd.img && \
curl -L -o /vmlinuz-amd64 http://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/images/pxeboot/vmlinuz; \
else \
/download_box.sh https://cloud.centos.org/centos/9-stream/s390x/images/CentOS-Stream-GenericCloud-9-$centos_version.s390x.qcow2 && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey I think we can change, but just want to not to mix those changes for x86 here. As this is already XXL, wanted to have that as a separate PR. I will create that as a follow-up PR for this one.

curl -L -o dockerize-linux-$BUILDARCH.tar.gz https://github.com/jwilder/dockerize/releases/download/$DOCKERIZE_VERSION/dockerize-linux-amd64-$DOCKERIZE_VERSION.tar.gz; \
else \
# Temporary till s390x support is upstreamed to dockerize (https://github.com/jwilder/dockerize/pull/200)
curl -L -o dockerize-linux-$BUILDARCH.tar.gz https://github.com/ibm-jitendra/kubevirt_pkgs/raw/main/dockerize-linux-s390x.tar.gz; \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey : No update as of now. We are tried to reach upstream owner on PR, but no luck. Now, Vamsi is trying to reach him on LinkedIn.

@@ -0,0 +1 @@
root=/dev/vda1 ro no_timer_check console=tty0 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were the defaults within the generic cloud image of centos9 for s390x. I've copied them to this file to mainly keep parity with x86 based kernel args, which I think exists for configuring args for kernel.

Will try to come up with a readme around this.

-device AC97 \
-uuid $(cat /proc/sys/kernel/random/uuid) \
${QEMU_ARGS}"
else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand you right, have s390x case in if block and x86_64 case in else block. I will change it accordingly.

@@ -252,21 +255,21 @@ func provisionCluster(cmd *cobra.Command, args []string) (retErr error) {
if err != nil {
return err
}
err = _cmd(cli, nodeContainer(prefix, nodeName), "if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images vagrant@192.168.66.101:/tmp/extra-pre-pull-images; fi", "copying /scripts/extra-pre-pull-images if existing")
err = _cmd(cli, nodeContainer(prefix, nodeName), fmt.Sprintf("if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images %s@192.168.66.101:/tmp/extra-pre-pull-images; fi", sshUser), "copying /scripts/extra-pre-pull-images if existing")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brianmcarey This PR changes are already part of my branch. But will check if I need to adjust it. As PR#1209 is quite big digesting it still.

@@ -14,13 +16,13 @@ export ISTIO_BIN_DIR="/opt/istio-${ISTIO_VERSION}/bin"
EOF
source $KUBEVIRTCI_SHARED_DIR/shared_vars.sh

# Install modules of the initrd kernel
# Install modules of the initrd kernel. These modules extend the kernel's functionality, providing support for various hardware devices, file systems, network protocols, KVM/virtualization, and other features.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove it. Added it as initially for my reference to understand the sequence of steps

dnf install -y "kernel-modules-$(uname -r)"

# Resize root partition
dnf install -y cloud-utils-growpart
if growpart /dev/vda 1; then
resize2fs /dev/vda1
if growpart /dev/vda 1; then #growpart adjusts the partition size to fill the available space on the disk
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove it

dnf install -y openvswitch2.16
#openvswitch for s390x is not available from the centos default repos.
if [ "$ARCH" == "s390x" ]; then
dnf install -y https://kojipkgs.fedoraproject.org//packages/openvswitch/2.16.0/2.fc36/s390x/openvswitch-2.16.0-2.fc36.s390x.rpm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, unfortunately.
I checked today also, couldn't find it else where.

Signed-off-by: chandramerla <Chandra.Merla@ibm.com>
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2024
@kubevirt-bot
Copy link
Contributor

PR needs rebase.

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-sigs/prow repository.

@chandramerla chandramerla mentioned this pull request Aug 21, 2024
8 tasks
Copy link
Contributor Author

@chandramerla chandramerla left a comment

Choose a reason for hiding this comment

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

@brianmcarey
I've addressed all your comments. As 1.28 is going away soon, created another branch where I've moved all these changes (+ addressing your comments) for k8s 1.30 provider. The PR for the same is : #1252

@@ -48,7 +48,7 @@ export KUBEVIRTCI_GOCLI_CONTAINER=quay.io/kubevirtci/gocli:latest
### start cluster

```bash
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in k8s-1.30-provider-slim-s390x branch

@@ -59,7 +59,7 @@ make cluster-up
#### start cluster with prometheus, alertmanager and grafana
To enable prometheus, please also export the following variables before running `make cluster-up`:
```bash
export KUBEVIRT_PROVIDER=k8s-1.30
export KUBEVIRT_PROVIDER=k8s-1.28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in k8s-1.30-provider-slim-s390x branch

curl -L -o /initramfs-amd64.img http://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/images/pxeboot/initrd.img && \
curl -L -o /vmlinuz-amd64 http://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/images/pxeboot/vmlinuz; \
else \
/download_box.sh https://cloud.centos.org/centos/9-stream/s390x/images/CentOS-Stream-GenericCloud-9-$centos_version.s390x.qcow2 && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the placeholder PR #1242 so that this can be further tracked there.

curl -L -o dockerize-linux-$BUILDARCH.tar.gz https://github.com/jwilder/dockerize/releases/download/$DOCKERIZE_VERSION/dockerize-linux-amd64-$DOCKERIZE_VERSION.tar.gz; \
else \
# Temporary till s390x support is upstreamed to dockerize (https://github.com/jwilder/dockerize/pull/200)
curl -L -o dockerize-linux-$BUILDARCH.tar.gz https://github.com/ibm-jitendra/kubevirt_pkgs/raw/main/dockerize-linux-s390x.tar.gz; \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for @vamsikrishna-siddu for the followup. Now changes are upstreamed.
This is addressed in k8s-1.30-provider-slim-s390x branch

@@ -0,0 +1 @@
root=/dev/vda1 ro no_timer_check console=tty0 console=ttyS0,115200n8 net.ifnames=0 biosdevname=0 crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this as separate commit in k8s-1.30-provider-slim-s390x branch, where in commit I've explained all the details.
6f3f133

-device AC97 \
-uuid $(cat /proc/sys/kernel/random/uuid) \
${QEMU_ARGS}"
else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is addressed in k8s-1.30-provider-slim-s390x branch

@@ -252,21 +255,21 @@ func provisionCluster(cmd *cobra.Command, args []string) (retErr error) {
if err != nil {
return err
}
err = _cmd(cli, nodeContainer(prefix, nodeName), "if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images vagrant@192.168.66.101:/tmp/extra-pre-pull-images; fi", "copying /scripts/extra-pre-pull-images if existing")
err = _cmd(cli, nodeContainer(prefix, nodeName), fmt.Sprintf("if [ -f /scripts/extra-pre-pull-images ]; then scp -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i vagrant.key -P 22 /scripts/extra-pre-pull-images %s@192.168.66.101:/tmp/extra-pre-pull-images; fi", sshUser), "copying /scripts/extra-pre-pull-images if existing")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't require to change this as in #1029 these function calls have not changed.

@@ -14,13 +16,13 @@ export ISTIO_BIN_DIR="/opt/istio-${ISTIO_VERSION}/bin"
EOF
source $KUBEVIRTCI_SHARED_DIR/shared_vars.sh

# Install modules of the initrd kernel
# Install modules of the initrd kernel. These modules extend the kernel's functionality, providing support for various hardware devices, file systems, network protocols, KVM/virtualization, and other features.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in k8s-1.30-provider-slim-s390x branch

dnf install -y "kernel-modules-$(uname -r)"

# Resize root partition
dnf install -y cloud-utils-growpart
if growpart /dev/vda 1; then
resize2fs /dev/vda1
if growpart /dev/vda 1; then #growpart adjusts the partition size to fill the available space on the disk
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in k8s-1.30-provider-slim-s390x branch

@@ -301,6 +315,11 @@ kubeadm_raw_ipv6=/tmp/kubeadm_ipv6.conf
kubeadm_manifest="/etc/kubernetes/kubeadm.conf"
kubeadm_manifest_ipv6="/etc/kubernetes/kubeadm_ipv6.conf"

# envsubst pkg is not available by default in s390x Architecture.
if [ "$arch" == "s390x" ]; then
dnf install -y gettext
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in k8s-1.30-provider-slim-s390x branch, by moving the line dnf install -y gettext to the end of provision.sh

@chandramerla
Copy link
Contributor Author

Closing this PR as 1.28 is no more part of k8s providers and also not these changes are moved to 1.30 provider in the PR: #1252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants