WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@dhaiducek
Copy link
Member

  • Replace matchLabels/matchExpressions with LabelSelector for simplicity
  • Clean up variable declarations throughout

mprahl
mprahl previously approved these changes Oct 29, 2024
mprahl
mprahl previously approved these changes Oct 29, 2024
@openshift-ci openshift-ci bot added the lgtm label Oct 29, 2024
- Replace `matchLabels`/`matchExpressions` with
`LabelSelector` for simplicity
- Clean up variable declarations throughout

Signed-off-by: Dale Haiducek <[email protected]>
Comment on lines +307 to +314
// If MatchLabels and MatchExpressions are nil, the resulting label selector
// matches all namespaces. This is to guard against that.
if t.LabelSelector == nil && len(t.Include) == 0 {
return []string{}, nil
}

labelSelector := parseToLabelSelector(t)
// List all namespaces by default, otherwise use provided LabelSelector
selector := labels.Everything()
Copy link
Member Author

@dhaiducek dhaiducek Oct 29, 2024

Choose a reason for hiding this comment

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

Apparently a nil LabelSelector matches no namespaces, so we have to make it explicit here now.

@dhaiducek
Copy link
Member Author

dhaiducek commented Oct 30, 2024

Flaky test failure 1; missing CSV:

• [FAILED] [65.815 seconds]
Testing OperatorPolicy Test reporting of unapproved version after installation [It] Should report a violation after the versions list is patched to exclude the current version [supports-hosted]
/home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case38_install_operator_test.go:3666

  Timeline >>
  STEP: Creating the parent object @ 10/29/24 21:34:30.751
  STEP: Creating the child object with the owner reference @ 10/29/24 21:34:30.87
  STEP: Verifying the child object exists @ 10/29/24 21:34:30.873
  STEP: Patching the versions field to exclude the installed version @ 10/29/24 21:34:30.875
  [FAILED] in [It] - /home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case38_install_operator_test.go:3671 @ 10/29/24 21:35:30.938
...
Debug:
NonCompliant; 
the policy spec is valid, 
the OperatorGroup matches what is required by the policy, 
the Subscription matches what is required by the policy, 
an InstallPlan to update to [quay-operator.v3.10.6] is available for approval but approval for [quay-operator.v3.10.6] is required, 
the ClusterServiceVersion required by the policy was not found, 
there are CRDs present for the operator, 
there are no relevant deployments because the ClusterServiceVersion is missing, 
CatalogSource was found"
...
  << Timeline

  [FAILED] Timed out after 60.001s.
  The function passed to Eventually failed at /home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case38_install_operator_test.go:154 with:
  Should have related object ClusterServiceVersion with name 'quay-operator.v3.10.6'
  Expected
      <bool>: false
  to be true
  In [It] at: /home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case38_install_operator_test.go:3671 @ 10/29/24 21:35:30.938

ref: https://github.com/open-cluster-management-io/config-policy-controller/actions/runs/11582544518/job/32245796587?pr=311#step:5:26173

@dhaiducek
Copy link
Member Author

dhaiducek commented Oct 30, 2024

Flaky failure 2; missing expected compliance event:

• [FAILED] [116.725 seconds]
Test compliance events of enforced policies that define a status [It] Should have the expected events
/home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case34_enforce_w_status_test.go:27

  Timeline >>
  STEP: Setting up the policy @ 10/29/24 22:05:50.065
  STEP: Creating the parent object @ 10/29/24 22:05:50.065
  STEP: Creating the child object with the owner reference @ 10/29/24 22:05:50.128
  STEP: Verifying the child object exists @ 10/29/24 22:05:50.13
  STEP: Checking there is a NonCompliant event on the policy @ 10/29/24 22:05:50.132
  STEP: Checking there are no Compliant events on the policy @ 10/29/24 22:05:55.205
  STEP: Updating the policy @ 10/29/24 22:06:20.206
  STEP: Checking there are no Compliant events created during the update flow @ 10/29/24 22:06:20.343
  STEP: Updating the nested policy to increment its generation @ 10/29/24 22:06:45.344
  STEP: Checking there is now a Compliant event on the policy @ 10/29/24 22:06:45.483
  [FAILED] in [It] - /home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case34_enforce_w_status_test.go:62 @ 10/29/24 22:07:45.484
  << Timeline

  [FAILED] Timed out after 60.001s.
  Expected
      <[]v1.Event | len:0, cap:0>: []
  not to be empty
  In [It] at: /home/runner/work/config-policy-controller/config-policy-controller/test/e2e/case34_enforce_w_status_test.go:62 @ 10/29/24 22:07:45.484

ref: https://github.com/open-cluster-management-io/config-policy-controller/actions/runs/11582544518/job/32247033611?pr=311#step:5:27561

@openshift-ci openshift-ci bot added the lgtm label Oct 30, 2024
@openshift-ci
Copy link

openshift-ci bot commented Oct 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhaiducek, mprahl

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-merge-bot openshift-merge-bot bot merged commit 90571b0 into open-cluster-management-io:main Oct 30, 2024
13 checks passed
@dhaiducek dhaiducek deleted the 14361-ns-nsselector branch October 30, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants