Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure that existing e2e tests for cluster-authentication-operator are executed in serial ?
xref: https://github.com/openshift/cluster-authentication-operator/blob/master/cmd/cluster-authentication-operator-tests-ext/main.go#L65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(^ not strictly related to the changes introduced in this PR :) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @p0lyn0mial
Tried checking the logs, understanding the framework. This is what i got.
Tests run serially by default in Go. You only need to explicitly opt-in to parallel execution
So currently we are executing cases from test/e2e directory by using Ginkgo framework to run the tests in serial way.
As per our OTE implementation we have assigned names like
OTE suite configuration with parallelism: 1, Tests run sequentially (serially), not concurrently
Dedicated serial CI job in OpenShift's test infrastructure in PR: https://github.com/openshift/release/pull/73999/changes
The tests in test/e2e/ modify cluster-wide global state (OAuth config, routes, templates, etc.). If they ran in parallel the test will fail.
Execution logs not from my PR
ANALYZING ACTUAL JUNIT XML FROM PR #829
========================================
TEST DURATIONS FROM junit_report.xml:
- TestCustomRouterCerts: 42.83s
- TestGitLabAsOIDCPasswordGrantCheck: 291.25s
- TestKeycloakAsOIDCPasswordGrantCheckAndGroupSync: 186.63s
- TestRouterCerts: 2.21s
- TestTemplatesConfig: 70.34s
- TestTokenInactivityTimeout: 2202.64s
└─ unset-inactivity-timeout-client-timeout: 610.57s
└─ with-client-timeout: 430.72s
└─ with-inactivity-timeout: 730.90s
└─ without-inactivity-timeout: 430.43s
mathematical proof from the actual CI logs that tests ran serially, not in parallel!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @p0lyn0mial Summary related to the tests under path test/e2e:
Can you PTAL and do let me know about your opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check how tests are executed by the CI job. I think the CI job uses.
Then compare this with how tests are executed for kas-o.
I think the difference is the use of the
-p=1flag.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @p0lyn0mial
Sure, Thanks. TBH i have taken reference from working service-ca-operator repo
Parallelism: 1 // Already there!
https://github.com/openshift/cluster-authentication-operator/blob/master/cmd/cluster-authentication-operator-tests-ext/main.go#L66
test-e2e: GO_TEST_FLAGS += -count 1 # Prevents caching and use count 1
https://github.com/openshift/cluster-authentication-operator/blob/master/Makefile#L49
env:
TEST_SUITE: openshift/cluster-authentication-operator/operator/serial
When you run tests via the OTE binary:
./cluster-authentication-operator-tests-ext run-suite openshift/cluster-authentication-operator/operator/serial
The binary's Parallelism: 1 setting controls execution, NOT go test flags.
The Makefile's -p 1 is only relevant for:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so, it looks like we will update the auth operator to execute the tests in parallel (xref: openshift/cluster-authentication-operator#833)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah agree, as per your suggestions here openshift/cluster-authentication-operator#833 (comment) i have updated to parallel. Will monitor the CI job once it gets merged.