-
Notifications
You must be signed in to change notification settings - Fork 5
ossm: add script for uploading built ztunnel to gs #14
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
base: release-1.27
Are you sure you want to change the base?
ossm: add script for uploading built ztunnel to gs #14
Conversation
Signed-off-by: Zuzana Miklankova <[email protected]>
ossm/ci/post-submit.sh
Outdated
| fi | ||
|
|
||
| tar czf ./out/rust/release/ztunnel.tar.gz ./out/rust/release/ztunnel | ||
| gsutil cp bazel-bin/envoy_tar.tar.gz "${ARTIFACTS_GCS_PATH}/ztunnel-alpha-${SHA}${ARCH_SUFFIX}.tar.gz" |
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 should copy ztunnel, not envoy binary
- are you sure we need to rename it to ztunnel-alpha? I'd check at https://github.com/istio/istio/blob/master/bin/build_ztunnel.sh#L127-L133
Take a look at those Istio scripts to see if it's really necessary to upload a binary artifact, or if Istio is capable of building ztunnel from source. Proxy is done that way because building proxy is really an expensive task.
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 should copy ztunnel, not envoy binary
oh, good catch!
Take a look at those Istio scripts to see if it's really necessary to upload a binary artifact, or if Istio is capable of building ztunnel from source. Proxy is done that way because building proxy is really an expensive task.
Good point, will do that.
Thanks.
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.
Hi @jwendell. Sorry it took me that long. From what I understood from istio repo, ztunnel is, similarly to proxy, pre-built and then pushed to gcloud. So I think the approach I proposed is correct.
No need to perform any prior creation. If a directory doesn't exist, it's created on demand. |
Signed-off-by: Zuzana Miklankova <[email protected]>
ossm/ci/post-submit.sh
Outdated
| fi | ||
|
|
||
| tar czf ./out/rust/release/ztunnel.tar.gz ./out/rust/release/ztunnel | ||
| gsutil cp ./out/rust/release/ztunnel.tar.gz "${ARTIFACTS_GCS_PATH}/ztunnel-${SHA}${ARCH_SUFFIX}.tar.gz" |
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 think we don't need to tar.gz it, AFAICT upstream uses the raw binary:
https://github.com/istio/ztunnel/blob/master/scripts/release.sh#L51
and
https://github.com/istio/istio/blob/master/bin/build_ztunnel.sh#L129
plus, I think we need to add the arch to the filename: https://github.com/istio/ztunnel/blob/master/scripts/release.sh#L44
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.
Should be fixed now, thanks.
Signed-off-by: Zuzana Miklankova <[email protected]>
ossm/ci/post-submit.sh
Outdated
| SHA="$(git rev-parse --verify HEAD)" | ||
|
|
||
| if [[ "$(uname -m)" == "aarch64" ]]; then | ||
| ARCH_SUFFIX="-arm64" |
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.
remove the -
ossm/ci/post-submit.sh
Outdated
| if [[ "$(uname -m)" == "aarch64" ]]; then | ||
| ARCH_SUFFIX="-arm64" | ||
| else | ||
| ARCH_SUFFIX="" |
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.
this should be set to amd64: https://github.com/istio/ztunnel/blob/master/scripts/release.sh#L23
Signed-off-by: Zuzana Miklankova <[email protected]>
| *) echo "unsupported architecture"; exit 1;; | ||
| esac | ||
|
|
||
| gsutil cp ./out/rust/release/ztunnel "${ARTIFACTS_GCS_PATH}/ztunnel-${SHA}-${ARCH}" |
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.
Sorry @zmiklank - I just noticed - We're not performing the build anywhere in this script, thus the file ./out/rust/release/ztunnel doesn't exist at this point.
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.
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.
The way it works is: After a PR is merged in ztunnel repository, a fresh new postsubmit job is triggered (copy-artifacts-gcs job in that file, which basically invokes this script (line 111 on that file). Thus, this script should perform a build and the upload the binary artifact.
See the proxy cousin: https://github.com/openshift-service-mesh/proxy/blob/release-1.28/ossm/ci/post-submit.sh#L17
This should be similar to https://github.com/openshift-service-mesh/proxy/blob/release-1.27/ossm/ci/post-submit.sh and should have the same purpose.
I have not done any steps to create
gs://maistra-prow-testing/ztunnelyet.