Skip to content
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

step-registry: add Origin E2E test step #6965

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

bbguimaraes
Copy link
Contributor

No description provided.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 31, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2020
@@ -202,3 +202,5 @@ tests:
steps:
cluster_profile: aws
workflow: ipi
test:
- ref: origin-e2e
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we likely want a workflow for this specific thing since everyone will have it

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

21 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

fi

# set up cloud-provider-specific env vars
KUBE_SSH_BASTION="$( oc --insecure-skip-tls-verify get node -l node-role.kubernetes.io/master -o 'jsonpath={.items[0].status.addresses[?(@.type=="ExternalIP")].address}' ):22"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding notes here so they're captured (as I'm reviewing the step registry)

This is wrong, bastion is an optional requirement for some steps, not required for all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, we should probably move all "SSH requiring" things out to their own step. It might even be appropriate for that always to be expecting the bastion. Will have to think about that.

export KUBE_SSH_USER=core
;;
azure4) export TEST_PROVIDER=azure;;
*) echo >&2 "Unsupported cluster type '${CLUSTER_TYPE}'"; exit 1;;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, the test suite correctly infers platform starting from 4.3. Remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are also executed in 4.1 and 4.2.

popd
fi

test_suite=openshift/conformance/parallel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should always be injected from a variable, all "standard" suites should be parameterized by the test (possibly by the workflow, but either way, this is wrong).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're working on adding test/step parameters.

fi

openshift-tests run "${test_suite}" \
--provider "${TEST_PROVIDER}" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be conditional based on whether TEST_PROVIDER is set. If TEST_PROVIDER was not set, this flag should not be set.

@@ -0,0 +1,12 @@
ref:
as: origin-e2e-test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to rename all these steps, this is not origin. Probably this should be openshift-test-e2e and successive variants will be openshift-test-e2e-serial openshift-test-e2e-disruptive, etc.

The name of our platform is openshift, okd and ocp are variants that are expected to run and pass the same test suites.

@bbguimaraes
Copy link
Contributor Author

@smarterclayton: can you open a PR with the changes you propose? It will be much faster than me, without any knowledge of the test suite, trying to understand your comments.

Also keep in mind this is almost a direct copy of the templates, so some of these issues exist there as well.

bbguimaraes added a commit to bbguimaraes/release that referenced this pull request Apr 30, 2020
bbguimaraes added a commit to bbguimaraes/release that referenced this pull request Apr 30, 2020
bbguimaraes added a commit to bbguimaraes/release that referenced this pull request Apr 30, 2020
bbguimaraes added a commit to bbguimaraes/release that referenced this pull request May 20, 2020
wking added a commit to wking/openshift-release that referenced this pull request Jun 16, 2020
…ST_SUITE

Taking advantage of openhsift/ci-tools@62e7498d66 (ci-operator
multi-stage: add step parameters, 2020-05-27, openshift/ci-tools#854).
This commit doesn't override the defaults yet, but a useful override
would be:

  TEST_COMMAND=run-upgrade
  TEST_SUITE=all

This replaces test-suite.txt, which landed in a2fd8c3
(step-registry: add Origin E2E test step, 2020-01-31, openshift#6965), and also
had no consumers.
wking added a commit to wking/openshift-release that referenced this pull request Jul 1, 2020
…step

These have been part of the e2e test step since that step landed in
a2fd8c3 (step-registry: add Origin E2E test step, 2020-01-31, openshift#6965).
But pushing manifests is more of an install thing than a test thing.
For example, if a user swaps in their own tests, we still want the
cluster to push insights if their cluster profile includes the token).

The logic I'm replacing was just handling the insights operator.  To
allow us to recycle the same logic for additional manifests, I've
carved out generic handling for ${CLUSTER_PROFILE_DIR} and
${SHARED_DIR}.  The day-2 naming avoids conflicts with the ipi/install
step that is already consuming ${SHARED_DIR}/manifest_*.yml

Glob expansion should have stable ordering; from POSIX [1]:

  If the pattern matches any existing filenames or pathnames, the
  pattern shall be replaced with those filenames and pathnames, sorted
  according to the collating sequence in effect in the current locale.

Ideally we'd update the cluster profiles themselves to store the
config in day-2-manifests-insights-live.yaml (via a
core-services/ci-secret-bootstrap/_config.yaml edit?).  But we can't
do that until we've removed or ported the old-style templates under
ci-operator/templates, so I'm leaving that off for now and keeping a
HACK check to copy the current profile's file into the newly-expected
target location.

The previous logic also used an error-ignoring '|| true' since at
least b887d20 (Deploy the insights secret for the support-operator
in all templates, 2019-06-04, openshift#3951).  That commit didn't motivate its
use, so I'm dropping it here.  If the step fails, figuring out the
culprit should be pretty straightforward.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_13_03
wking added a commit to wking/openshift-release that referenced this pull request Mar 21, 2021
… openshift-tests

The end-to-end step originated without the background/wait pattern in
a2fd8c3 (step-registry: add Origin E2E test step, 2020-01-31, openshift#6965).
But as described in 4472ace
(ci-operator/templates/openshift/installer: Restore backgrounded
'create cluster', 2019-01-23, openshift#2680), we want the background-and-wait
pattern to make openshift-tests a child process that will receive
TERMs via this step's existing 'trap' handler.  openshift-tests can
take hours, and when the step gets a TERM, we want to quickly pass
that through to the slow children, so they exit, so the step can exit,
so we have plenty of time for asset uploading and subsequent gather
and teardown steps.  That gives us the resources we need to figure out
why the test was abnormally slow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants