Prow script for pilot-multicluster-e2e test#5516
Prow script for pilot-multicluster-e2e test#5516tiswanso wants to merge 13 commits intoistio:masterfrom
Conversation
|
/test istio-pilot-multicluster-e2e |
|
e2e multicluster failed because the gke/gcp user associated with the prow job gke doesn't have permission to create firewall rules 👎 ... need to figure out whether the test jobs' user can get those permissions bumped or if we can make mason do it. |
prow/istio-pilot-multicluster-e2e.sh
Outdated
There was a problem hiding this comment.
Why are we running this with auth disabled. If this is temporary to get the easiest environment to prove out the process then I think it is fine and we can increment in a later PR, but with auth should be the default case.
There was a problem hiding this comment.
Sure... was just trying to get it going with less complexity. I'll change it.
But for the CI the more pressing problem is the prow env's lack of permissions to set firewall rules in the projects/istio-testing GCP project.
Codecov Report
@@ Coverage Diff @@
## master #5516 +/- ##
======================================
+ Coverage 71% 71% +1%
======================================
Files 369 369
Lines 32122 32040 -82
======================================
- Hits 22607 22577 -30
+ Misses 8588 8532 -56
- Partials 927 931 +4
Continue to review full report at Codecov.
|
|
trying a run with removing use of |
|
/test istio-pilot-multicluster-e2e |
|
Run failed with same permissions error when trying with removing use of |
9336a14 to
45a16c4
Compare
45a16c4 to
3f77a92
Compare
c33c542 to
e1b1004
Compare
prow/e2e-suite.sh
Outdated
There was a problem hiding this comment.
This should be done by the framework.
There was a problem hiding this comment.
@sebastienvas -- by "framework" are you meaning Mason?
The main thing that needs to happen in unsetup_clusters is that the firewall rule needs to be removed from the project since Mason won't do that just by destroying clusters. The other stuff I added in so I could rerun the tests over and over without always deleting and recreating clusters in my own GKE proj.
There was a problem hiding this comment.
I meant to go e2e framework.
There was a problem hiding this comment.
The e2e framework won't be able to delete the gcloud firewall rule so something like unsetup_clusters is needed for GKE projs usage.
Also, the service-account stuff was done by cluster_lib.sh and unsetup_clusters removes that from the remote cluster. There's a PR to put the service account create inside the helm chart used to setup the remote cluster. But I'd like to get this merged and update the script later along with a fix to the e2e multicluster.go after the helm merges.
prow/cluster_lib.sh
Outdated
There was a problem hiding this comment.
you are actually running gcloud on istio-testing here, not on the project where the cluster are created.
|
You can take my patch from #6549 |
8558f99 to
0373644
Compare
|
@sebastienvas -- I merged in your version of the fix for the proj from #6549 |
0373644 to
c05f944
Compare
prow/cluster_lib.sh
Outdated
There was a problem hiding this comment.
Please use {} around variable names for consistency.
prow/cluster_lib.sh
Outdated
There was a problem hiding this comment.
Same comment here about {}
prow/istio-pilot-multicluster-e2e.sh
Outdated
There was a problem hiding this comment.
Not sure what this is ?
There was a problem hiding this comment.
leftover comment ... will remove
I should be able to remove that GIT_SHA stuff as well since it's setup in e2e-suite.sh
|
/test istio-pilot-multicluster-e2e |
prow/mason_lib.sh
Outdated
There was a problem hiding this comment.
I know I did that but could please you change cat "${info_path}" | head -n 1 to head -n 1 "${info_path}"
sebastienvas
left a comment
There was a problem hiding this comment.
Please see comments
-Add functions for multi-cluster setup to cluster_lib
gcloud IAM roles seem to have instance-template.* as a higher privilidge than instance.list|get.
2d0cc28 to
2ffe236
Compare
|
New changes are detected. LGTM label has been removed. |
|
rebased and lint failures finally went away :-) |
|
@hklai this is ready to go in. Can we merge it ? |
|
This will be merge once the 1.0 code freeze is over (hopefully next week), when we re-enable the merge bot. And if this is needed for 1.0, please rebase and merge to the release-1.0 branch instead of master. |
|
Sorry for dropping the ball here. Had an offline discussion with @sebastienvas and was wondering if this should go to releaae-1.0 instead of master. @tiswanso is this still needed? |
|
@hklai -- Yes... this is still needed. It could go into release-1.0 instead. It is only effecting testing. |
|
@tiswanso: The following tests failed, say
DetailsInstructions 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. I understand the commands that are listed here. |
|
I wasn't able to reopen this PR so I recreated the PR -- #7376 |
Script called by CI to runs multicluster e2e test. Uses mason to get GKE resources in
the same GCP project. The boskos resource_type already creates multiple GKE clusters. Use
kubectl config get-contextsto determine all k8s clusters to use for the multicluster env.The script can be run with pre-allocated clusters by having a pre-setup
kubeconfig for multiple clusters. It will choose the pilot cluster to
be the first cluster in the
kubectl config get-contextsor the context set in thePILOT_CLUSTERenv var.Example for use with 2 precreated GKE clusters and designating "gke_istio-ci-testing_us-east1-c_cluster-1" context as the cluster to host the istio control-plane (pilot, etc).
for non-GKE clusters: