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

Plumbing Setup for Additional CI tests for Per-feature Flag Testing Matrix #1629

Closed
Tracked by #7177
JeromeJu opened this issue Oct 10, 2023 · 4 comments · Fixed by #1803
Closed
Tracked by #7177

Plumbing Setup for Additional CI tests for Per-feature Flag Testing Matrix #1629

JeromeJu opened this issue Oct 10, 2023 · 4 comments · Fixed by #1803
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@JeromeJu
Copy link
Member

JeromeJu commented Oct 10, 2023

Feature request

Per TEP-0138, we are going to add additional CI tests for Per-feature Flag Testing Matrix. The testing matrix could be referred from:
https://github.com/tektoncd/community/blob/main/teps/0138-decouple-api-and-feature-versioning.md#additional-test-combinations

There are two ways handling this:

  • One is to add an additional CI tests as a standalone test, probably named as pull-tekton-pipeline-per-feature-flags-matrix-test.
    • If we want to go this way we might as well want to decide whether we want to put it in nightly or the regular CIs that runs per PR build. My understanding is that since each per-feature flags shall be independent from each other, we might just want to do this nightly.
  • To add the additional testing matrix to the current alpha/beta/stable integration tests. This would add to the time for the existing CI and potentially more time and flakes for the existing CI.

This issue is raised to discuss and aim to reach consensus on the decisions of:

  • Whether we want to add this as a standalone test?
  • Shall we integrate it in the existing CIs per build or the nightly?

cc @chitrangpatel @afrittoli @dibyom @vdemeester

Use case

With us switching to the per-feature flags, we would need to make sure the combinations of each per feature flag could work as a group. So a testing matrix as proposed in TEP-0138 would provide enough confidence for us adding more feature flags.

@JeromeJu JeromeJu added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 10, 2023
@chitrangpatel
Copy link
Contributor

Based on the estimates for how long it would take to run these tests, I vote for per-build test since it's totally feasible. We don't really have a good rollback mechanism for nightlies. I'd rather catch these sooner.

@vdemeester
Copy link
Member

What makes a "standalone test" ?
Asking this because, to run those different "matrix of test", we definitely don't need to provision different cluster, nor rebuild the images multiples times (ko apply …). Today, two different CI checks (test-integration and test-integration-alpha, …) do that though — they build the entire project (without cache), they use their own kind cluster, …

  • We can have one cluster, and change the configuration over and over.
  • We also don't necessarily have to run all tests for all the combination on pull-request…
  • … but we can do that on some nightly though

We probably want to refine the way we do CI on tektoncd/pipeline a bit, and find some ways to group those, optimize a little bit the build/test time, etc.. (tektoncd/community#1083 or #1264 could help a little bit). This is also where it gets tricky to have the CI setup for a project to be in another one.

@JeromeJu
Copy link
Member Author

JeromeJu commented Oct 11, 2023

Thanks for the opinion @vdemeester . I am generally onboard and in favor of the Opinionated CI. But I am afraid the current context might fall between the implementation of that refinement and the current WIP of per feature flag CI setups :/

What makes a "standalone test" ?

I was trying to refer to another CI test similar as pull-tekton-pipeline-integration-test & pull-tekton-pipeline-alpha-integration-test. For example, the new one might probably get named as pull-tekton-pipeline-per-feature-flags-test.

I agree that we might not need additional clusters for setting up the per feature combination test.

Agree on both of the following:

  • We can have one cluster, and change the configuration over and over.
  • We also don't necessarily have to run all tests for all the combination on pull-request…

In terms of the decision of whether we would like to have the test in nightly, I do get the point of having this in nightly since theoretically per feature flags shall be independent to each other. Meanwhile I also agree with @chitrangpatel 's statement that nightly test results might not provide timely feedback for rollbacks.

@JeromeJu
Copy link
Member Author

JeromeJu commented Feb 6, 2024

It has been a while since the previous discussion for this. Now that we have more per-feature flags that came in, let's actually set it up as a nightly for now - given it takes around only ~15 minutes for 4 flags. We can estimate to take ~8 keys within the timeframe of 60mins.

JeromeJu added a commit to JeromeJu/plumbing that referenced this issue Feb 12, 2024
This commit adds the prow setup for feature flag tests.

/kind misc
fixes: tektoncd#1629
JeromeJu added a commit to JeromeJu/plumbing that referenced this issue Feb 12, 2024
This commit adds the prow setup for feature flag tests.

/kind misc
fixes: tektoncd#1629
tekton-robot pushed a commit that referenced this issue Feb 13, 2024
This commit adds the prow setup for feature flag tests.

/kind misc
fixes: #1629
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants