CNTRLPLANE-1137: Add ci for cilium e2e test - #71383
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Two minor comments (see inline). Otherwise I think it looks good.
With this job we'd be able to run Hypershift E2E test suite on AKS. That's fine. But there would be a mismatch between AKS and other platforms (AWS, Baremetal, Kubevirt) where we run Conformance tests for Cilium and Calico.
I will leave this to someone else to consider if this is ok. But later, if we want to get to the same level of support for AKS and other platforms we'd need to run the conformance tests as well. Thanks
| OCP_IMAGE_N3: release:n3minor | ||
| OCP_IMAGE_N4: release:n4minor | ||
| env: | ||
| AUTH_THROUGH_CERTS: "true" |
There was a problem hiding this comment.
I suppose EXTERNAL_CNI_PROVIDER needs to be set here. Otherwise it won't run with Cilium.
| # Run test with external CNI provider if specified | ||
| if [[ -n "${EXTERNAL_CNI_PROVIDER:-}" ]]; then | ||
| run_e2e_test "--e2e.external-cni-provider=${EXTERNAL_CNI_PROVIDER}" | ||
| fi |
There was a problem hiding this comment.
Could you use the same approach like the rest of variables? For example, there's this (see below). The N1_NP_VERSION_TEST_ARGS is always passed to the command but sometimes it is empty, sometimes not.
N1_NP_VERSION_TEST_ARGS=""
if [[ ${OCP_IMAGE_N1} != "${OCP_IMAGE_LATEST}" ]]; then
N1_NP_VERSION_TEST_ARGS="--e2e.n1-minor-release-image=${OCP_IMAGE_N1}"
fi
In our new case, it would be something like this:
EXTERNAL_CNI_PROVIDER_ARGS=""
if [[ -n ${EXTERNAL_CNI_PROVIDER:-} ]]; then
EXTERNAL_CNI_PROVIDER_ARGS="--e2e.external-cni-provider=${EXTERNAL_CNI_PROVIDER}"
fi
And then it would be used like this:
hack/ci-test-e2e.sh -test.v \
-test.run=${CI_TESTS_RUN:-} \
-test.parallel=20 \
--e2e.platform=Azure \
--e2e.azure-credentials-file=/etc/hypershift-ci-jobs-azurecreds/credentials.json \
--e2e.pull-secret-file=/etc/ci-pull-credentials/.dockerconfigjson \
--e2e.base-domain=hypershift.azure.devcluster.openshift.com \
--e2e.azure-location=${HYPERSHIFT_AZURE_LOCATION} \
--e2e.oidc-issuer-url=${AZURE_OIDC_ISSUER_URL} \
--e2e.sa-token-issuer-private-key-path=${AZURE_SA_TOKEN_ISSUER_KEY_PATH} \
${EXTERNAL_CNI_PROVIDER_ARGS:-} \ //<--------------------- HERE -------
${EXTERNAL_DNS_ARGS:-} \
${AKS_ANNOTATIONS:-} \
${N1_NP_VERSION_TEST_ARGS:-} \
${N2_NP_VERSION_TEST_ARGS:-} \
${N3_NP_VERSION_TEST_ARGS:-} \
${N4_NP_VERSION_TEST_ARGS:-} \
${MI_ARGS:-} \
${DP_ARGS:-} \
${AZURE_MULTI_ARCH_PARAMS:-} \
--e2e.azure-encryption-key-id=${AKS_KMS_KEY} \
--e2e.azure-kms-credentials-secret-name=${AKS_KMS_CREDENTIALS_SECRET} \
${MARKETPLACE_IMAGE_PARAMS} \
--e2e.latest-release-image="${OCP_IMAGE_LATEST}" \
--e2e.previous-release-image="${OCP_IMAGE_PREVIOUS}" &
There was a problem hiding this comment.
Updated, thanks reviewing.
|
@wewang58: This pull request references CNTRLPLANE-1137 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
30fb9af to
6772385
Compare
|
Issues in openshift/release go stale after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
|
Stale issue in openshift/release rot after 15d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
| MARKETPLACE_IMAGE_PARAMS="--e2e.azure-marketplace-publisher ${HYPERSHIFT_AZURE_MARKETPLACE_IMAGE_PUBLISHER} --e2e.azure-marketplace-offer ${HYPERSHIFT_AZURE_MARKETPLACE_IMAGE_OFFER} --e2e.azure-marketplace-sku ${HYPERSHIFT_AZURE_MARKETPLACE_IMAGE_SKU} --e2e.azure-marketplace-version ${HYPERSHIFT_AZURE_MARKETPLACE_IMAGE_VERSION}" | ||
| fi | ||
| EXTERNAL_CNI_PROVIDER_ARGS="" | ||
| if [[ -n ${EXTERNAL_CNI_PROVIDER:-} ]]; then |
There was a problem hiding this comment.
I suppose this also needs to atomatically set --network-type=Other ?
Otherwise the condition if platform != hyperv1.NonePlatform && hostedCluster.Spec.Networking.NetworkType != hyperv1.Other { here won't match and it will try to "validate" the cluster even though the CNI is Cilium https://github.com/openshift/hypershift/pull/7423/files#diff-65fe136e12a7c2756931254c4f076ef0b5bc990b0fc13a8407647f421ddfa328R128
6772385 to
01943fd
Compare
|
Rotten issues in openshift/release close after 15d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. DetailsIn 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-sigs/prow repository. |
|
/reopen |
|
@wewang58: This pull request references CNTRLPLANE-1137 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.22.0" version, but no target version was set. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@wewang58: Reopened this PR. DetailsIn 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-sigs/prow repository. |
|
@mgencur Recently busy with jobset operator GA test, when have time will update the pr. |
01943fd to
e07706d
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wewang58 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aks-cilium |
|
@wewang58: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
e07706d to
8b30474
Compare
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aks-cilium |
|
@wewang58: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
8b30474 to
bcddc4e
Compare
|
/pj-rehearse pull-ci-openshift-hypershift-main-e2e-aks-cilium |
|
@wewang58: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 67 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@wewang58: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
|
Rotten issues in openshift/release close after 15d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. DetailsIn 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-sigs/prow repository. |
Add ci for e2e test codes: openshift/hypershift#7077