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

Enable multiarch builds for pr-creator image #3617

Merged

Conversation

chandramerla
Copy link
Contributor

What this PR does / why we need it:
This PR updates base image (golang) version and Go version to newer versions and mainly enables multi-arch images for pr-creator.

Multi-arch images for pr-creator are needed as now with #3566 we want to move PR creation part into periodic-kubevirtci-bump-centos-base-s390x. So that after x86 centos images are built, followed by s390x builds and manifest-list, we will create the PR in the s390x machine.

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:

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

@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 Aug 30, 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.

@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.

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.

Can you update the presubmit as well?

Then I will be able to run rehearsals to test it.

@chandramerla
Copy link
Contributor Author

Can you update the presubmit as well?

Then I will be able to run rehearsals to test it.

@brianmcarey I've updated project-infra-presubmits.yaml

@brianmcarey
Copy link
Member

/rehearse

@kubevirt-bot
Copy link
Contributor

Rehearsal jobs created for this PR:

rehearsal-build-pr-creator-image

You can trigger rehearsal for all jobs by commenting either /rehearse or /rehearse all
on this PR.

For a specific PR you can comment /rehearse {job-name}.

For a list of jobs that you can rehearse you can comment /rehearse ?.

@brianmcarey
Copy link
Member

@chandramerla The resource limits will have to removed from the prowjobs as we are doing emulated builds. I can see the rehearsal is running very slowly due to strict CPU limits.

@chandramerla
Copy link
Contributor Author

@chandramerla The resource limits will have to removed from the prowjobs as we are doing emulated builds. I can see the rehearsal is running very slowly due to strict CPU limits.

I've removed the resource section completely from the spec section, so I believe it falls back to defaults which I assure are higher :)

The last job run failed (unexpected disconnect) during git checkout of test-infra repo. Could it be due to network issues or github rate limits?

  • git clone https://github.com/kubernetes/test-infra.git
    Cloning into 'test-infra'...
    error: RPC failed; curl 92 HTTP/2 stream 5 was not closed cleanly: CANCEL (err 8)
    error: 6239 bytes of body are still expected
    fetch-pack: unexpected disconnect while reading sideband packet
    fatal: early EOF
    fatal: fetch-pack: invalid index-pack output
    Error: building at STEP "RUN set -x && git clone https://github.com/kubernetes/test-infra.git && cd ./test-infra && git checkout e4d87dae72dbf1754625cbf112afc535307c1db4 && /usr/local/bin/runner.sh /bin/sh -ce 'go build -o ./robots/pr-creator/pr-creator ./robots/pr-creator/main.go' && mv ./robots/pr-creator/pr-creator /usr/local/bin/ && chmod +x /usr/local/bin/pr-creator && /usr/local/bin/runner.sh /bin/sh -ce 'go clean -cache -modcache' && cd .. && rm -rf ./test-infra": while running runtime: exit status 128
  • EXIT_VALUE=128

@chandramerla
Copy link
Contributor Author

/retest

@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:

/retest

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

/rehearse

@kubevirt-bot
Copy link
Contributor

Rehearsal jobs created for this PR:

rehearsal-build-pr-creator-image

You can trigger rehearsal for all jobs by commenting either /rehearse or /rehearse all
on this PR.

For a specific PR you can comment /rehearse {job-name}.

For a list of jobs that you can rehearse you can comment /rehearse ?.

@brianmcarey
Copy link
Member

/rehearse

@kubevirt-bot
Copy link
Contributor

Rehearsal jobs created for this PR:

rehearsal-build-pr-creator-image

You can trigger rehearsal for all jobs by commenting either /rehearse or /rehearse all
on this PR.

For a specific PR you can comment /rehearse {job-name}.

For a list of jobs that you can rehearse you can comment /rehearse ?.

@chandramerla chandramerla force-pushed the enable-multiarch-builds-for-pr-creator branch from 5ec81c0 to dd1468b Compare September 6, 2024 11:30
@kubevirt-bot kubevirt-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL and removed size/S labels Sep 6, 2024
@chandramerla chandramerla force-pushed the enable-multiarch-builds-for-pr-creator branch from dd1468b to 5ec81c0 Compare September 6, 2024 11:49
@kubevirt-bot kubevirt-bot added size/S and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL labels Sep 6, 2024
@kubevirt-bot kubevirt-bot added size/M and removed size/S labels Sep 6, 2024
@chandramerla chandramerla force-pushed the enable-multiarch-builds-for-pr-creator branch from c0a88de to b16ee49 Compare September 6, 2024 12:34
@chandramerla chandramerla marked this pull request as ready for review September 6, 2024 12:36
@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 Sep 6, 2024
@chandramerla
Copy link
Contributor Author

Last run of job rehearsal-build-pr-creator-image failed as arm64 build output path for labels-checker ie., ./bazel-out/aarch64-fastbuild/bin/robots/cmd/labels-checker/labels-checker_/labels-checker
is different from amd64 path ie., ./bazel-out/k8-fastbuild/bin/robots/cmd/labels-checker/labels-checker_/labels-checker

So, Now I've updated bazel based build of labels-checker with go native build and tested it locally and multi-arch build of pr-creator is successful. As bazel is not supported in s390x, I've replaced bazel based build with go native build, there the above problem won't be there anyway as it generates the output in the given path across archs.

Could someone please trigger reharse job again?

@brianmcarey
Copy link
Member

/rehearse

@kubevirt-bot
Copy link
Contributor

Rehearsal jobs created for this PR:

rehearsal-build-pr-creator-image

You can trigger rehearsal for all jobs by commenting either /rehearse or /rehearse all
on this PR.

For a specific PR you can comment /rehearse {job-name}.

For a list of jobs that you can rehearse you can comment /rehearse ?.

@brianmcarey
Copy link
Member

/test all

… and labels-checker build to go instead of bazel

As bazel isn't supported on s390x, changed the bazel based build of labels-checker to go native way of building

Signed-off-by: chandramerla <156769580+chandramerla@users.noreply.github.com>
@chandramerla chandramerla force-pushed the enable-multiarch-builds-for-pr-creator branch from b16ee49 to 0617dbb Compare September 11, 2024 11:18
@brianmcarey
Copy link
Member

/test all

securityContext:
privileged: true
resources:
requests:
memory: "16Gi"
Copy link
Member

Choose a reason for hiding this comment

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

Can you please just re-add the memory requests here ? This just ensures that is gets enough RAM on the node.

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 You mean only memory requests ie.,
resources: requests: memory: "16Gi"

without adding back the cpu requests and memory+cpu limits ?

P.S: As we discussed earlier in DM, I will add -a flag to ./publish_multiarch_image.sh once testing is complete as this is pre-submit job.

Copy link
Member

Choose a reason for hiding this comment

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

@brianmcarey You mean only memory requests ie., resources: requests: memory: "16Gi"

without adding back the cpu requests and memory+cpu limits ?

Yes without the cpu requests and memory+cpu limits - thanks

Copy link
Member

Choose a reason for hiding this comment

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

P.S: As we discussed earlier in DM, I will add -a flag to ./publish_multiarch_image.sh once testing is complete as this is pre-submit job.

I am ok with leaving the presubmit as is for now - this image doesn't get updated that often so doing the full build of the three images in the presubmit should not be too costly.

Copy link
Contributor Author

@chandramerla chandramerla Sep 12, 2024

Choose a reason for hiding this comment

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

Better I will add -a. That way I can keep the same resource requests 16Gi, than changing to 29Gi, as multi-arch build for all archs needs more resources. Also to be consistent with other image builds in presubmits. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I'm leaving this as is (without -a) and adding 29Gi for memory.

Later before merge I can change - to add -a and keeping only memory request to 16Gi.

…blish multi-arch images

Signed-off-by: chandramerla <156769580+chandramerla@users.noreply.github.com>
…lti-arch images

Signed-off-by: chandramerla <156769580+chandramerla@users.noreply.github.com>
@chandramerla chandramerla force-pushed the enable-multiarch-builds-for-pr-creator branch from 0617dbb to 773a121 Compare September 12, 2024 08:49
@brianmcarey
Copy link
Member

/rehearse

@kubevirt-bot
Copy link
Contributor

Rehearsal jobs created for this PR:

rehearsal-build-pr-creator-image

You can trigger rehearsal for all jobs by commenting either /rehearse or /rehearse all
on this PR.

For a specific PR you can comment /rehearse {job-name}.

For a list of jobs that you can rehearse you can comment /rehearse ?.

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.

/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brianmcarey

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:

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 12, 2024
@kubevirt-bot kubevirt-bot merged commit 6773146 into kubevirt:main Sep 12, 2024
7 checks passed
@kubevirt-bot
Copy link
Contributor

@chandramerla: Updated the job-config configmap in namespace kubevirt-prow at cluster default using the following files:

  • key project-infra-postsubmits.yaml using file github/ci/prow-deploy/files/jobs/kubevirt/project-infra/project-infra-postsubmits.yaml
  • key project-infra-presubmits.yaml using file github/ci/prow-deploy/files/jobs/kubevirt/project-infra/project-infra-presubmits.yaml

In response to this:

What this PR does / why we need it:
This PR updates base image (golang) version and Go version to newer versions and mainly enables multi-arch images for pr-creator.

Multi-arch images for pr-creator are needed as now with #3566 we want to move PR creation part into periodic-kubevirtci-bump-centos-base-s390x. So that after x86 centos images are built, followed by s390x builds and manifest-list, we will create the PR in the s390x machine.

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:

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants