MGMT-15598: Add a parameter to manifest list to disable filter #5498

paul-maidment · 2023-09-18T16:38:47Z

Recently we made a change to the manifest list to filter out the system generated manifests when listing them.

This has led to issues with data gathering for triage which requires full access to all manifests, system generated or otherwise.

I have introduced a boolean parameter IncludeSystemGenerated which defaults to false. When set to true, the manifest list endpoint will show all manifests, irrespective of how they were created.

It is intended that we will make a change in assisted-test-infra that will send this parameter.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci-robot · 2023-09-18T16:38:51Z

filanov · 2023-09-18T16:54:06Z

swagger.yaml

+          name: include_system_generated
+          description: Include system generated manifests in results? Default is false.
+          type: boolean
+          required: true


should be optional no?

filanov · 2023-09-18T16:55:22Z

/assign @avishayt

codecov · 2023-09-18T16:58:33Z

Codecov Report

Merging #5498 (4a8a61b) into master (2b6e4c4) will increase coverage by 0.02%.
Report is 10 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5498      +/-   ##
==========================================
+ Coverage   67.64%   67.66%   +0.02%     
==========================================
  Files         232      232              
  Lines       34099    34089      -10     
==========================================
+ Hits        23065    23067       +2     
+ Misses       8971     8965       -6     
+ Partials     2063     2057       -6

Files	Coverage Δ
internal/manifests/manifests.go	`77.14% <100.00%> (ø)`

... and 4 files with indirect coverage changes

paul-maidment · 2023-09-18T18:08:55Z

/retest

paul-maidment · 2023-09-19T07:18:14Z

/retest

filanov · 2023-09-19T17:28:54Z

internal/manifests/manifests.go

@@ -176,7 +176,7 @@ func (m *Manifests) ListClusterManifestsInternal(ctx context.Context, params ope
 		if err != nil {
 			return nil, err
 		}
-		if isUserManifest {
+		if isUserManifest || (params.IncludeSystemGenerated != nil && *params.IncludeSystemGenerated) {


Suggested change

if isUserManifest || (params.IncludeSystemGenerated != nil && *params.IncludeSystemGenerated) {

if isUserManifest || swag.BoolValue(params.IncludeSystemGenerated) {

paul-maidment · 2023-09-20T07:18:20Z

/retest

paul-maidment · 2023-09-26T10:33:20Z

/retest

openshift-ci-robot · 2023-09-26T10:49:59Z

/retest-required

Remaining retests: 0 against base HEAD 932f8e0 and 2 for PR HEAD cb2ec59 in total

Recently we made a change to the manifest list to filter out the system generated manifests when listing them. This has led to issues with data gathering for triage which requires full access to all manifests, system generated or otherwise. I have introduced a boolean parameter IncludeSystemGenerated which defaults to false. When set to true, the manifest list endpoint will show all manifests, irrespective of how they were created. It is intended that we will make a change in assisted-test-infra that will send this parameter.

openshift-ci · 2023-09-27T10:41:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: filanov, paul-maidment

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:

~~OWNERS~~ [filanov,paul-maidment]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-09-27T11:09:16Z

/retest-required

Remaining retests: 0 against base HEAD f09956b and 2 for PR HEAD 4a8a61b in total

openshift-ci-robot · 2023-09-27T20:00:18Z

/retest-required

Remaining retests: 0 against base HEAD 8450824 and 1 for PR HEAD 4a8a61b in total

openshift-ci · 2023-09-28T01:01:18Z

@paul-maidment: all tests passed!

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.

…hift#5498) Recently we made a change to the manifest list to filter out the system generated manifests when listing them. This has led to issues with data gathering for triage which requires full access to all manifests, system generated or otherwise. I have introduced a boolean parameter IncludeSystemGenerated which defaults to false. When set to true, the manifest list endpoint will show all manifests, irrespective of how they were created. It is intended that we will make a change in assisted-test-infra that will send this parameter.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 18, 2023

openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 18, 2023

openshift-ci bot requested review from gamli75 and vrutkovs September 18, 2023 16:49

openshift-ci bot added api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 18, 2023

filanov reviewed Sep 18, 2023

View reviewed changes

openshift-ci bot assigned avishayt Sep 18, 2023

paul-maidment force-pushed the MGMT-15598 branch from b4fe659 to 9fe34b5 Compare September 18, 2023 17:32

filanov reviewed Sep 19, 2023

View reviewed changes

paul-maidment force-pushed the MGMT-15598 branch from 9fe34b5 to cb2ec59 Compare September 20, 2023 07:17

filanov approved these changes Sep 26, 2023

View reviewed changes

openshift-ci bot assigned filanov Sep 26, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2023

paul-maidment force-pushed the MGMT-15598 branch from cb2ec59 to 059be44 Compare September 26, 2023 10:51

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2023

paul-maidment force-pushed the MGMT-15598 branch from 059be44 to 300c1cd Compare September 26, 2023 11:01

paul-maidment force-pushed the MGMT-15598 branch from 300c1cd to 4a8a61b Compare September 27, 2023 10:34

filanov approved these changes Sep 27, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2023

openshift-merge-robot merged commit 4658458 into openshift:master Sep 28, 2023
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-15598: Add a parameter to manifest list to disable filter #5498

MGMT-15598: Add a parameter to manifest list to disable filter #5498

paul-maidment commented Sep 18, 2023

openshift-ci-robot commented Sep 18, 2023 •

edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

filanov Sep 18, 2023

filanov commented Sep 18, 2023

codecov bot commented Sep 18, 2023 •

edited

paul-maidment commented Sep 18, 2023

paul-maidment commented Sep 19, 2023

filanov Sep 19, 2023

paul-maidment commented Sep 20, 2023

paul-maidment commented Sep 26, 2023

openshift-ci-robot commented Sep 26, 2023

openshift-ci bot commented Sep 27, 2023

openshift-ci-robot commented Sep 27, 2023

openshift-ci-robot commented Sep 27, 2023

openshift-ci bot commented Sep 28, 2023

	if isUserManifest \|\| (params.IncludeSystemGenerated != nil && *params.IncludeSystemGenerated) {
	if isUserManifest \|\| swag.BoolValue(params.IncludeSystemGenerated) {

MGMT-15598: Add a parameter to manifest list to disable filter #5498

MGMT-15598: Add a parameter to manifest list to disable filter #5498

Conversation

paul-maidment commented Sep 18, 2023

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Sep 18, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

filanov Sep 18, 2023

Choose a reason for hiding this comment

filanov commented Sep 18, 2023

codecov bot commented Sep 18, 2023 • edited

Codecov Report

paul-maidment commented Sep 18, 2023

paul-maidment commented Sep 19, 2023

filanov Sep 19, 2023

Choose a reason for hiding this comment

paul-maidment commented Sep 20, 2023

paul-maidment commented Sep 26, 2023

openshift-ci-robot commented Sep 26, 2023

openshift-ci bot commented Sep 27, 2023

openshift-ci-robot commented Sep 27, 2023

openshift-ci-robot commented Sep 27, 2023

openshift-ci bot commented Sep 28, 2023

openshift-ci-robot commented Sep 18, 2023 •

edited by openshift-ci bot

codecov bot commented Sep 18, 2023 •

edited