Skip to content

NTO: add PAO e2e ci test lanes and fix unit tests target #27140

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
Mar 28, 2022
Merged

NTO: add PAO e2e ci test lanes and fix unit tests target #27140

merged 1 commit into from
Mar 28, 2022

Conversation

yanirq
Copy link
Contributor

@yanirq yanirq commented Mar 21, 2022 •
edited
Loading

This PR includes:

This PR does not include test lanes for:

  • performance profile creator
  • PAO render command

@openshift-ci openshift-ci bot requested review from dagrayvid and jmencak March 21, 2022 12:50
@yanirq
Copy link
Contributor Author

yanirq commented Mar 21, 2022

/cc @fromanirh @cynepco3hahue

ffromani
ffromani reviewed Mar 22, 2022
View reviewed changes
Copy link
Contributor

@ffromani ffromani left a comment

Choose a reason for hiding this comment

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

LGTM, the job definition looks consistent with what we do now for PAO. I'll review the makefile targets separately.

@jmencak
Copy link
Contributor

jmencak commented Mar 22, 2022

Looks good to me. Once the Makefile changes in openshift/cluster-node-tuning-operator#322 merge, this should be good to go. Right now, the current Makefile misses the new targets, so the tests will keep failing.

@jmencak
Copy link
Contributor

jmencak commented Mar 23, 2022

One more thing. We may want to add unit tests as part of this PR, but if you prefer, this can be a separate one. I.e. make test-unit

@yanirq
Copy link
Contributor Author

yanirq commented Mar 23, 2022

One more thing. We may want to add unit tests as part of this PR, but if you prefer, this can be a separate one. I.e. make test-unit

It seems there is a already a test set for unit tests , just not pointing to the right target in the existing Makefile:

release/ci-operator/config/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-release-4.11.yaml

Lines 46 to 50 in 379092b

tests:
- as: unit
commands: make test
container:
from: src

I can change that to the right target (make test-unit) which will run unit tests for all the components (both NTO and PAO)

@jmencak
Copy link
Contributor

jmencak commented Mar 24, 2022

I can change that to the right target (make test-unit) which will run unit tests for all the components (both NTO and PAO)

Yes, let's do this.

@yanirq yanirq changed the title NTO: add PAO e2e ci test lanes NTO: add PAO e2e ci test lanes and fix unit tests target Mar 24, 2022
@yanirq
NTO: add PAO e2e ci test lanes and fix unit tests
93ef496 
- Add PAO e2e functional test lane.
- Fix unit tests Makefile target.

Signed-off-by: Yanir Quinn <yquinn@redhat.com>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 24, 2022

@yanirq: 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/pj-rehearse 93ef496 link false /test pj-rehearse
ci/rehearse/openshift/cluster-node-tuning-operator/master/e2e-gcp-pao 93ef496 link unknown /test pj-rehearse
ci/rehearse/openshift/cluster-node-tuning-operator/release-4.11/e2e-gcp-pao 93ef496 link unknown /test pj-rehearse
ci/rehearse/openshift/cluster-node-tuning-operator/release-4.12/e2e-gcp-pao 93ef496 link unknown /test pj-rehearse

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.

jmencak
jmencak approved these changes Mar 28, 2022
View reviewed changes
Copy link
Contributor

@jmencak jmencak left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 28, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmencak, yanirq

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 Mar 28, 2022
@openshift-merge-robot openshift-merge-robot merged commit 2693301 into openshift:master Mar 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 28, 2022

@yanirq: Updated the following 6 configmaps:

  • ci-operator-4.11-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cluster-node-tuning-operator-release-4.11.yaml using file ci-operator/config/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-release-4.11.yaml
  • ci-operator-4.12-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cluster-node-tuning-operator-release-4.12.yaml using file ci-operator/config/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-release-4.12.yaml
  • job-config-master-presubmits configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cluster-node-tuning-operator-master-presubmits.yaml using file ci-operator/jobs/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-master-presubmits.yaml
  • job-config-4.11 configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cluster-node-tuning-operator-release-4.11-presubmits.yaml using file ci-operator/jobs/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-release-4.11-presubmits.yaml
  • job-config-4.12 configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cluster-node-tuning-operator-release-4.12-presubmits.yaml using file ci-operator/jobs/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-release-4.12-presubmits.yaml
  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-cluster-node-tuning-operator-master.yaml using file ci-operator/config/openshift/cluster-node-tuning-operator/openshift-cluster-node-tuning-operator-master.yaml

In response to this:

This PR includes:

This PR does not include test lanes for:

  • performance profile creator
  • PAO render command

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
Reviewers

@ffromani ffromani ffromani left review comments

@jmencak jmencak jmencak approved these changes

@dagrayvid dagrayvid Awaiting requested review from dagrayvid

@cynepco3hahue cynepco3hahue Awaiting requested review from cynepco3hahue

Assignees

@jmencak jmencak

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yanirq @jmencak @ffromani @openshift-merge-robot