Skip to content

cli: Generate each owners field separately #38987

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 17, 2025
Merged

cli: Generate each owners field separately #38987

merged 1 commit into from
Apr 17, 2025

Conversation

joestringer
Copy link
Member

Rather than encoding all owners into junit with one line per test
scenario, pull all unique owners+tests into a map first and then encode
each unique owner+test as a separate property.

This should de-duplicate some of the owners results we see in the final
dashboard representation of this data.

@joestringer
cli: Generate each owners field separately
b1b849b 
Rather than encoding all owners into junit with one line per test
scenario, pull all unique owners+tests into a map first and then encode
each unique owner+test as a separate property.

This should de-duplicate some of the owners results we see in the final
dashboard representation of this data.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer requested a review from a team as a code owner April 16, 2025 20:33
@joestringer joestringer added the release-note/ci This PR makes changes to the CI. label Apr 16, 2025
@joestringer joestringer requested a review from christarazi April 16, 2025 20:33
@github-actions github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary labels Apr 16, 2025
@joestringer
Copy link
Member Author

joestringer commented Apr 16, 2025 •
edited
Loading

Example snippet of junit XML before via local testing:

          <testcase name="echo-ingress-from-other-client-deny" classname="connectivity test" status="skipped" time="0">
              <properties>
                  <property name="owner" value="@cilium/sig-agent (pod-to-pod)"></property>
                  <property name="owner" value="@cilium/sig-agent (pod-to-pod)"></property>
                  <property name="owner" value="@cilium/ci-structure (client-to-client)"></property>
              </properties>
              <skipped message="echo-ingress-from-other-client-deny skipped"></skipped>
          </testcase>

After:

          <testcase name="echo-ingress-from-other-client-deny" classname="connectivity test" status="skipped" time="0">
              <properties>
                  <property name="owner" value="@cilium/ci-structure (client-to-client)"></property>
                  <property name="owner" value="@cilium/sig-agent (pod-to-pod)"></property>
              </properties>
              <skipped message="echo-ingress-from-other-client-deny skipped"></skipped>
          </testcase>

I also noticed in some full CI runs that we ended up with many duplicates like this:

          <testcase name="client-egress-l7-method" classname="connectivity test" status="passed" time="32.743188731000004">
              <properties>
                  <property name="owner" value="@cilium/sig-agent (pod-to-pod-with-endpoints), @cilium/aws (.github/workflows/conformance-eks.yaml), @cilium/ipsec (.github/workflows/conformance-eks.yaml), @cilium/ci-structure (.github/workflows/conformance-eks.yaml)"></property>
                  <property name="owner" value="@cilium/sig-agent (pod-to-pod-with-endpoints), @cilium/aws (.github/workflows/conformance-eks.yaml), @cilium/ipsec (.github/workflows/conformance-eks.yaml), @cilium/ci-structure (.github/workflows/conformance-eks.yaml)"></property>
              </properties>
          </testcase>

After this PR, I expect this to deduplicate like:

          <testcase name="client-egress-l7-method" classname="connectivity test" status="passed" time="32.743188731000004">
              <properties>
                  <property name="owner" value="@cilium/sig-agent (pod-to-pod-with-endpoints)"></property>
                  <property name="owner" value="@cilium/aws (.github/workflows/conformance-eks.yaml)"></property>
                  <property name="owner" value="@cilium/ipsec (.github/workflows/conformance-eks.yaml)"></property>
                  <property name="owner" value="@cilium/ci-structure (.github/workflows/conformance-eks.yaml)"></property>
              </properties>
          </testcase>

Fortunately the ingester at https://github.com/isovalent/corgi can handle both forms so we should only need to make this change in the CLI.

@joestringer
Copy link
Member Author

/test

@joestringer joestringer enabled auto-merge April 16, 2025 20:37
@joestringer joestringer disabled auto-merge April 17, 2025 20:23
@joestringer joestringer merged commit 204197e into main Apr 17, 2025
334 of 339 checks passed
@joestringer joestringer deleted the pr/joe/split-owners-properties branch April 17, 2025 20:23
tklauser
tklauser reviewed Apr 17, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tklauser tklauser tklauser left review comments

@christarazi christarazi christarazi approved these changes

Assignees
No one assigned
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary release-note/ci This PR makes changes to the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joestringer @tklauser @christarazi