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
test: Improve gke/{select,release}-cluster.sh
scripts
#11173
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a029658
to
69eb72b
Compare
This comment has been minimized.
This comment has been minimized.
69eb72b
to
8040be5
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
1ec2e5f
to
d434acf
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
test-me-please |
d434acf
to
3d7e249
Compare
@nebril I'm happy with this one now, please take a look when you get a chance. Once it's in, I'll start new clusters rolling in. |
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.
Good changes overall, there is one issue though - get-cluster-version.sh
was used in gke jenkinsfile, please change https://github.com/cilium/cilium/blob/master/jenkinsfiles/ginkgo-gke.Jenkinsfile#L141 to use new scheme of getting k8s version.
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, I totally missed the first file in review xD
test-gke |
retest-ginkgo |
3d7e249
to
87fab3c
Compare
test-gke |
test-gke |
1 similar comment
test-gke |
@nebril did it fail before you re-ran it? I was hoping to have a look at the logs... do you have a link handy, or was it an unrelated flake? |
@errordeveloper it was an unrelated flake while building docker image: https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/745/execution/node/78/log/ |
@nebril I see. I'm seeing a different thing now:
Should we just ignore that or it could be not completely random? |
This comment has been minimized.
This comment has been minimized.
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.
Small nit, can't we merge both scripts and use an bash argument instead to pick which operation we want to do in the clusters? like test/gke/cluster.sh release
and test/gke/cluster.sh select
@aanm thanks for the review! To be honest, I am not sure what |
Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
- use explicit paths - make better use of `gcloud` features, reduce external dependencies - replace `get-cluster-version.sh` with a version file - use region instead of zone to enable regional spread and avoid availibity issue due to a zonal outage; namely we can create clusters in any region/zone and the Jenkins job is not tied into any particular one - list node pools instead of assuming default pool Signed-off-by: Ilya Dmitrichenko <errordeveloper@gmail.com>
87fab3c
to
c18f5f9
Compare
Here's what failed last time in the GKE suite:
I am not sure what's going in, will rebase and re-run. |
test-me-please |
GKE:
Others have different failures:
and
|
All failures were unrelated to changes, we need this to go forward with stabilising GKE ci, merging it as is. |
@nebril thanks for merging! 🚀 |
gcloud
features, reduce external dependenciesget-cluster-version.sh
with a version file