-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Run a ingress-gce script #29210
Run a ingress-gce script #29210
Conversation
/assign @panslava |
/ok-to-test |
looks ok, I don't have permission to approve though |
It seems to me that you have forgotten to add the file |
It is a part of another commit: kubernetes/ingress-gce#2046 |
I see, thanks for the clarification! I was looking for this file in https://github.com/kubernetes/test-infra/tree/7750f9b325a0ababc84e560e143bc282f6780d9f/hack 😅 |
/lgtm but I am not the owner of this repo |
/assign @dims |
/assign @aojea |
- ALL_ARCH=amd64 arm64 | ||
- CONTAINER_BINARIES=e2e-test | ||
- --env=ALL_ARCH="amd64 arm64" | ||
- --env=ARCH=$(dpkg --print-architecture) |
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 don't think this is correct, ARCH should define the target arch, not the one the job is running
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.
Do you know how to get a target ARCH in the script that test-infra runs?
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.
Ok, then maybe I misunderstood it, why do you need the current arch for crosscompiling?
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.
@aojea In the Makefile we see an ARCH = amd64. To override it we need to pass the desired architecture.
Otherwise the script will try to build ALL_ARCH on the same machine. It will fail or, even worse, it will build something on a wrong arch and push a new image that will be hard to debug 🤣
We could change Makefile itself and run the $(dpkg --print-architecture)
command inside of it. I am not sure it makes the difference.
but I am also not sure if running --env=ARCH=$(dpkg --print-architecture)
will have the same result as running export ARCH=$(dpkg --print-architecture)
inside building container
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.
ok, got it, but the infra doesn't have arm workers at this moment so we can omit that step by now, besides is more correct as you say
we can also migrate the job, this will result in something like
kubernetes/ingress-gce:
- name: ci-ingress-gce-image-push
decorate: true
labels:
preset-service-account: "true"
preset-k8s-ssh: "true"
preset-dind-enabled: "true"
spec:
containers:
- image: gcr.io/k8s-staging-test-infra/kubekins-e2e:v20230328-17e539c80f-master
command:
- runner.sh
- hack/run-make-cmd.sh
securityContext:
privileged: true
env:
- name: ALL_ARCH
value: "amd64 arm64"
- name: BINARY
value: "e2e-test"
annotations:
testgrid-dashboards: sig-network-ingress-gce-e2e
testgrid-tab-name: ingress-gce-image-push
testgrid-alert-email: kubernetes-sig-network-test-failures@googlegroups.com
It is tricky to run a list of commands inside config yamls. To do that it's better to run a script. The script builds & pushes the e2e-test image on multiarch. Signed-off-by: Elina Akhmanova <elinka@google.com>
/lgtm Thanks 🤞 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, code-elinka 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 |
@code-elinka: Updated the
In response to this:
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. |
@code-elinka @aojea You guys should be sleeping now! |
this is the OSS hour XD |
It is tricky to run a list of commands inside
config yamls. To do that it's better to run
a script. The script builds & pushes the e2e-test
image on multiarch.