Skip to content

Add must_gather tests#943

Open
marioferh wants to merge 1 commit into
openshift-kni:release-4.10from
marioferh:mustgather_tests_4_10
Open

Add must_gather tests#943
marioferh wants to merge 1 commit into
openshift-kni:release-4.10from
marioferh:mustgather_tests_4_10

Conversation

@marioferh

Copy link
Copy Markdown
Contributor

Add must_gather tests, manual cherry-pick from NTO openshift/cluster-node-tuning-operator#442

Signed-off-by: Mario Fernandez mariofer@redhat.com

@openshift-ci

openshift-ci Bot commented Aug 30, 2022

Copy link
Copy Markdown
Contributor

@marioferh: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Details

In response to this:

Add must_gather tests

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.

@coveralls

coveralls commented Aug 30, 2022

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 2640

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 60.532%

Totals Coverage Status
Change from base Build 2631: 0.0%
Covered Lines: 1592
Relevant Lines: 2630

💛 - Coveralls

@marioferh

Copy link
Copy Markdown
Contributor Author

/test e2e-gcp-operator-upgrade

@yanirq

yanirq commented Oct 27, 2022

Copy link
Copy Markdown
Member

if this is still needed for master we will need to create a must gather test lane instead of e2e-gcp

@marioferh

Copy link
Copy Markdown
Contributor Author

It is needed, but also good for future PR's.

@yanirq

yanirq commented Nov 2, 2022

Copy link
Copy Markdown
Member

@marioferh we actually need this cherry pick for 4.11 and master branch here dont we ?
At least until we move must gather to a different repo

}

mgImage := "quay.io/openshift-kni/performance-addon-operator-must-gather"
mgTag := "4.11-snapshot"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

notice that you are using here the 4.10 branch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

right. Fixed.

Signed-off-by: Mario Fernandez <mariofer@redhat.com>
@marioferh marioferh force-pushed the mustgather_tests_4_10 branch from 364cd38 to 0397579 Compare November 3, 2022 10:21
@openshift-ci

openshift-ci Bot commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: marioferh
Once this PR has been reviewed and has the lgtm label, please assign fromanirh for approval by writing /assign @fromanirh in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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 commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

@marioferh: 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/e2e-gcp 0397579 link true /test e2e-gcp
ci/prow/e2e-gcp-operator-upgrade 0397579 link true /test e2e-gcp-operator-upgrade

Full PR test history. Your PR dashboard.

Details

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.

@ffromani ffromani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the general direction seems good, some questions inside


It("Verify PAO cluster resources are captured", func() {
profile, _ := profiles.GetByNodeLabels(testutils.NodeSelectorLabels)
if profile == nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need this for anything else?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this PR is a cherry-pick. We can continue in the original one: #943

}
//replace peformance.yaml for profile.Name when data is generated in the node
ClusterSpecificFiles := []string{
"cluster-scoped-resources/performance.openshift.io/performanceprofiles/performance.yaml",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

...perhaps to construct the names here (and below)?

})

It("Verify hardware related information are captured", func() {
nodeSpecificFiles := []string{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I'd move this down below closer to the function using this slice. Now it is unnecessarily far from the calling site.

if err != nil {
return err
}
if !info.IsDir() && info.Name() == "sysinfo.tgz" {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: this should probably be a constant (in a must-gather package?)

return nil
}

func Untar(root string, snapshotName string) error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we can/should reuse ghw code here

}

mgImage := "quay.io/openshift-kni/performance-addon-operator-must-gather"
mgTag := "4.10-snapshot"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this be constructed from PAO version, or, in general, NOT be hardcoded?

@ffromani

Copy link
Copy Markdown
Member

scratch my comments, I didn't notice this is a cherry pick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants