Use istioctl-stage in default tests #350
Conversation
| @@ -33,7 +33,8 @@ fi | |||
|
|
|||
| # Exports $HUB, $TAG, and $ISTIOCTL_URL | |||
| source greenBuild.VERSION | |||
| echo "Using artifacts from HUB=${HUB} TAG=${TAG} ISTIOCTL_URL=${ISTIOCTL_URL}" | |||
| ISTIOCTL_STAGE_URL=${ISTIOCTL_URL}-stage | |||
| echo "Using artifacts from HUB=${HUB} TAG=${TAG} ISTIOCTL_STAGE_URL=${ISTIOCTL_STAGE_URL}" | |||
There was a problem hiding this comment.
echo ISTIOCTL_STAGE_URL here
There was a problem hiding this comment.
Hmm, I am not quiet sure what you mean. I think I do echo the ISTIOCTL_STAGE_URL here :)
There was a problem hiding this comment.
Can we just update ${ISTIOCTL_URL} to ${ISTIOCTL_URL}-stage so it will just be a one line change after istio/istio#2904 (comment) is addressed?
There was a problem hiding this comment.
We are using ISTIOCTL_URL somewhere else in this script, so it would be good to keep it.
| @@ -33,7 +33,8 @@ fi | |||
|
|
|||
| # Exports $HUB, $TAG, and $ISTIOCTL_URL | |||
| source greenBuild.VERSION | |||
| echo "Using artifacts from HUB=${HUB} TAG=${TAG} ISTIOCTL_URL=${ISTIOCTL_URL}" | |||
| ISTIOCTL_STAGE_URL=${ISTIOCTL_URL}-stage | |||
| echo "Using artifacts from HUB=${HUB} TAG=${TAG} ISTIOCTL_STAGE_URL=${ISTIOCTL_STAGE_URL}" | |||
There was a problem hiding this comment.
echo ISTIOCTL_STAGE_URL here
|
I suppose this needs to be changed again after istio/istio#2904 (comment) is addressed. cc @mattdelco |
|
If the non-docker.io istioctl is really needed then I'm having a hard time seeing how this ever passed for any daily build since the whole time it's been hard-coded to docker.io. I'm having a hard time making sense of what the test is doing. An alternative for getting the URL is to source the ISTIOCTL_URL that's in the tarball's istio.VERSION, but offhand it looks like the test uses the regular tarball instead of the "TESTONLY" tars. |
|
@mattdelco Sorry for the late response. In this PR, we directly feed a URL to download the istioctl into the e2e framework. So it doesn't matter what in the tarball's istio.VERSION. |
|
@yutongz The point I was trying to make is that the test might be more useful (i.e., test what's actually shipped in a release) if it paid attention to the URL in istio.VERSION rather than be directly fed the URL to istioctl. This partly came to mind because I saw the ${DAILY_BUILD}-linux.tar.gz that might not account for the "TESTONLY" tars, so I thought it might be cleaner to have the test only be told which of the tars to use (rather than need to be told this plus which istioctl to use). |
This PR should fix #352 for now.