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

WIP: Dynamic variants in Component Readiness #1628

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dgoodwin
Copy link
Contributor

@dgoodwin dgoodwin commented Apr 22, 2024

This is a PoC to explore how we could get component readiness working with dynamic variants instead of a fixed set as we do today.

The general approach outlined thus far:

  • we control the query for component readiness in code, so programatically inject columns for each variant requested in the API call into the select, join, and group by portions of the query. No view is required for this, chatgpt revealed a way to do it with standard joins.
  • switch to a more dynamic method of reading the column values back out of the bigquery results, as we don't have fixed columns anymore that will map to a static go struct.
  • variants move to a map[string]string on the ComponentTestIdentification.
  • because we use ComponentTestIdentification as a map key, and structs containing map fields cannot be used as map keys, switch to using a string representation (json serialized right now). Will need to watch computation cost but the implication of doing a few thousand serializations/deserializations of small json structs is likely insignificant.

@dgoodwin dgoodwin changed the title variant dynamic query WIP: Dynamic variants in Component Readiness Apr 22, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2024
@openshift-ci openshift-ci bot requested review from neisw and stbenjam April 22, 2024 18:05
Copy link
Contributor

openshift-ci bot commented Apr 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgoodwin

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2024
@@ -472,6 +477,29 @@ func (c *componentReportGenerator) getJobRunTestStatusFromBigQuery() (apitype.Co
}

func (c *componentReportGenerator) getCommonTestStatusQuery() (string, string, []bigquery.QueryParameter) {

// Simulate a list of varaints that were requested, this would be specified in the API request / UI in the real world...
variants := []string{"Network", "Upgrade", "Architecture", "Platform"}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

groupBy query param becomes the above array

Additional includeVariant query param comes in with Installer:hypershift for example. When includes are specified we inject a join and a where, but not a groupby for that variant name.

Copy link
Contributor

openshift-ci bot commented Apr 23, 2024

@dgoodwin: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/build 57778fe link true /test build
ci/prow/unit 57778fe link true /test unit
ci/prow/e2e 57778fe link true /test e2e
ci/prow/images 57778fe link true /test images
ci/prow/lint 57778fe link true /test lint

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.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2024
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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.

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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants