Adding Alibaba platform to CI #20841
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
kwoodson
force-pushed
the
alibaba_ci
branch
3 times, most recently
from
1d88a94
to
7ef1dba
Compare
ci-operator/step-registry/ipi/aws/ipi-aws-workflow.metadata.json
Outdated
Show resolved
Hide resolved
openshift-ci
bot
added
the
do-not-merge/hold
kwoodson
force-pushed
the
alibaba_ci
branch
2 times, most recently
from
2635370
to
85a12b6
Compare
|
just dropping a heads up here, i have #20668 open to add the basic files for the MAPI provider |
openshift-ci
bot
added
the
needs-rebase
openshift-ci
bot
removed
the
needs-rebase
openshift-ci
bot
added
the
needs-rebase
openshift-ci
bot
removed
the
needs-rebase
kwoodson
force-pushed
the
alibaba_ci
branch
4 times, most recently
from
cf73b9c
to
639eee8
Compare
kwoodson
force-pushed
the
alibaba_ci
branch
4 times, most recently
from
86d067a
to
1e3edf8
Compare
openshift-ci
bot
removed
the
needs-rebase
kwoodson
force-pushed
the
alibaba_ci
branch
2 times, most recently
from
872c1cc
to
4e1dcb1
Compare
|
/test step-registry-shellcheck |
|
|
||
| pushd /tmp | ||
| wget https://aliyuncli.alicdn.com/aliyun-cli-linux-latest-amd64.tgz -O aliyun-cli.tgz | ||
| tar zxvf aliyun-cli.tgz |
There was a problem hiding this comment.
Not sure how worried you are about running this freshly downloaded executable. There are certainly many other steps that do similar stuff. But if you wanted to pin a specific version and confirm that the binary you received had not been corrupted, this is an example of one way to check.
There was a problem hiding this comment.
This was spawned from our previous discussion. I filed an issue and it will be addressed aliyun/aliyun-cli#401
| /tmp/aliyun --config-path "${ALIBABA_CLI_CREDENTIALS_FILE}" configure set --region "${LEASED_RESOURCE}" | ||
|
|
||
|
|
||
| cat "${SHARED_DIR}/alibaba-instance-ids.txt" >> "${TMPDIR}/node-provider-IDs.txt" |
There was a problem hiding this comment.
I think you want to decouple the aliyun setup from the ID collection. Something like:
if test -f "${SHARED_DIR}/alibaba-instance-ids.txt"
then
cat "${SHARED_DIR}/alibaba-instance-ids.txt" >> "${TMPDIR}/node-provider-IDs.txt"
fi
# If you really feel motivated, you could exit 0 here if there were no IDs in the assembled node-provider-IDs.txt , but I think that's unlikely enough that we can ignore it. The existing AWS and GCP console gatherers don't bother.
pushd /tmp
# blah, blah, install aliyun
| do | ||
| echo "Gathering console logs for ${INSTANCE_ID}" | ||
| /tmp/aliyun --config-path "${ALIBABA_CLI_CREDENTIALS_FILE}" ecs GetInstanceConsoleOutput --RegionId "${LEASED_RESOURCE}" --InstanceId "$INSTANCE_ID" | jq -r '.ConsoleOutput' | base64 -d > "${ARTIFACT_DIR}/${INSTANCE_ID}" & | ||
| wait "$!" |
There was a problem hiding this comment.
Not sure where the ID came from, but the rehearsal failed:
$ curl -s https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_release/20841/rehearse-20841-pull-ci-openshift-installer-master-e2e-alibaba/1485730605567053824/build-log.txt | grep gather-alibabacloud-console
INFO[2022-01-24T22:56:36Z] Running step e2e-alibaba-gather-alibabacloud-console.
INFO[2022-01-24T22:57:06Z] Logs for container test in pod e2e-alibaba-gather-alibabacloud-console:
INFO[2022-01-24T22:57:06Z] Step e2e-alibaba-gather-alibabacloud-console failed after 30s.
Link to job on registry info site: https://steps.ci.openshift.org/job?org=openshift&repo=installer&branch=master&test=e2e-alibaba, "e2e-alibaba" post steps failed: "e2e-alibaba" pod "e2e-alibaba-gather-alibabacloud-console" failed: the pod ci-op-cm47ntwf/e2e-alibaba-gather-alibabacloud-console failed after 20s (failed containers: test): ContainerFailed one or more containers exited
Link to step on registry info site: https://steps.ci.openshift.org/reference/gather-alibabacloud-consolewith:
Gathering console logs for i-0xih3nnep3txd8c4asyo
�[1;31mERROR: SDK.ServerError
ErrorCode: InvalidParameter
Recommend: https://error-center.aliyun.com/status/search?Keyword=InvalidParameter&source=PopGw
RequestId: 78DC8847-01E5-3D1C-BB4A-6F9D2E4C247D
Message: The instanceId provided is not valid
�[0m
Maybe a bootstrap node? Maybe we need to accept failures here for the subset that are "I don't recognize that instance ID"?
I also don't understand the empty tmp-censor254417089 file here.
There was a problem hiding this comment.
I believe this instance ID is the bootstrap. Since it gets torn down the console log query fails. Will attempt to fix this morning.
| #PLATFORM=$(/tmp/yq r "${SHARED_DIR}/install-config.yaml" '.platform' | awk -F':' '{print$1}') | ||
| #if [[ "${PLATFORM}" == "alibabacloud" ]]; | ||
| #then | ||
| # STORAGE="20Gi"; |
There was a problem hiding this comment.
I don't feel to set on making this sizing conditional on the chosen platform, but I don't think we need to keep these dead comments around. Can we either revive this approach, or drop the commented code and explain why we skipped making it platform-specific?
| export PROVIDER_ARGS="-provider=alibabacloud -gce-zone=us-east-1" | ||
| # TODO: make openshift-tests auto-discover this from cluster config | ||
| REGION="$(oc get -o jsonpath='{.status.platformStatus.alibabacloud.region}' infrastructure cluster)" | ||
| export TEST_PROVIDER="{\"type\":\"alibabacloud\",\"region\":\"${REGION}\",\"multizone\":true,\"multimaster\":true}" |
There was a problem hiding this comment.
Dropping export for now might avoid.
E0124 22:54:06.535535 281 test_context.go:503] Unknown provider "alibabacloud". The following providers are known: aws azure baremetal gce gke ibmcloud kubemark kubevirt local openstack ovirt skeleton vsphere
until origin can be taught about the new provider type.
There was a problem hiding this comment.
| curl -O https://aliyuncli.alicdn.com/aliyun-cli-linux-3.0.83-amd64.tgz | ||
| tar -xzf aliyun-cli-linux-3.0.83-amd64.tgz | ||
| mkdir -p bin/ | ||
| mv aliyun bin/ |
There was a problem hiding this comment.
we should drop this until we find a test-case that actually needs the aliyun binary in the PATH.
| echo "No kubeconfig; skipping providerID extraction." | ||
| fi | ||
|
|
||
| if test -f "${SHARED_DIR}/alibaba-instance-ids.txt" && test -s "${SHARED_DIR}/alibaba-instance-ids.txt" |
There was a problem hiding this comment.
$ man test | grep -A1 -- '-[sf]'
-f FILE
FILE exists and is a regular file
--
-s FILE
FILE exists and has a size greater than zeroThe only case I see where -s is true but -f is not would be if the file was a symlink, and I don't think we need to care about excluding that. Can we just use:
if test -s "${SHARED_DIR}/alibaba-instance-ids.txt"here? Or am I missing something?
| cat "${SHARED_DIR}/alibaba-instance-ids.txt" >> "${TMPDIR}/node-provider-IDs.txt" | ||
| else | ||
| echo "No alibaba-instance-ids.txt; skipping console log retrieval." | ||
| exit 0 |
There was a problem hiding this comment.
nit: inconsistent tab/space indent. Also, in this case, we don't want to exit, because we may have content in node-provider-IDs.txt from the kubeconfig-powered oc listings. Just logging the fact that alibaba-instance-ids.txt was missing/empty should be sufficient, and then we can continue on to install aliyun. In the case where node-provider-IDs.txt turns out to be empty, the aliyun install is a waste, but 🤷 not so expensive that I think it needs a custom guard.
There was a problem hiding this comment.
Will check for node-provider-IDs before installing aliyun.
|
/approve Providing |
openshift-ci
bot
added
the
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kwoodson, petr-muller, rvanderp3 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 |
kwoodson
removed
the
do-not-merge/hold
|
@kwoodson: Updated the following 3 configmaps:
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. |
|
@kwoodson: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
@kwoodson this is now a required job for every installer patch, despite the fact that it has never once passed (including on this review). |
Looks like it was required by accident. If someone creates a PR to make it optional I will approve it, otherwise I will create one myself in a few hours. |
This pull request attempts to add support for the Alibaba platform to the CI.