Skip to content
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.

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

add OCP job to knmstate #304

Merged
merged 1 commit into from
Jan 15, 2020
Merged

Conversation

phoracek
Copy link
Member

We want to test against D/S too. The underlying system and the
NetworkManager version there may be different. Also, OCP lane seems
to be more stable than OKD.

This also mark the OKD lane as optional since it is consistently
not working.

Signed-off-by: Petr Horacek [email protected]

@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/M labels Jan 14, 2020
@oshoval
Copy link
Contributor

oshoval commented Jan 14, 2020

LGTM
lets run it here ?

@phoracek
Copy link
Member Author

We first need to get nmstate/kubernetes-nmstate#337 in

@qinqon
Copy link
Contributor

qinqon commented Jan 15, 2020

/test check-prow-config

- release-\d+\.\d+
annotations:
fork-per-release: "true"
always_run: false
Copy link
Contributor

Choose a reason for hiding this comment

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

true ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It takes a lot of time. Let's just trigger it manually when a PR is ready. It is also "optional" until we make sure it is stable enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough

- "/usr/local/bin/runner.sh"
- "/bin/sh"
- "-c"
- "yum install -y jq && cat $QUAY_PASSWORD | docker login --username $(cat $QUAY_USER) --password-stdin=true quay.io && automation/check-patch.e2e-ocp.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nicer if we instdall jq at the provider not here :-/, so we don't have to do yum install at every lane.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw maybe we dont even need to install
on the ocp-4.3 lane of kubevirt we dont install, lets not install and see what happens ?
it would fail directly at the begining of cluster-up since it wont have jq to parse the credits
so we will see it

Copy link
Member Author

Choose a reason for hiding this comment

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

Per Or's suggestion, I'm dropping the install for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am using jq from a container here kubevirt/kubevirtci#231 so it does no tdepend on underlaying system.

We want to test against D/S too. The underlying system and the
NetworkManager version there may be different. Also, OCP lane seems
to be more stable than OKD.

This also mark the OKD lane as optional since it is consistently
*not* working.

Signed-off-by: Petr Horacek <[email protected]>
@phoracek
Copy link
Member Author

/retest

1 similar comment
@phoracek
Copy link
Member Author

/retest

@qinqon
Copy link
Contributor

qinqon commented Jan 15, 2020

/hold

Let's wait for kubevirt/kubevirtci#231 so we don't have to install jq.

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 15, 2020
@qinqon
Copy link
Contributor

qinqon commented Jan 15, 2020

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 15, 2020
@qinqon
Copy link
Contributor

qinqon commented Jan 15, 2020

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2020
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 15, 2020
@qinqon
Copy link
Contributor

qinqon commented Jan 15, 2020

/test check-prow-config

@kubevirt-bot kubevirt-bot merged commit c0dd1c9 into kubevirt:master Jan 15, 2020
@kubevirt-bot
Copy link
Contributor

@phoracek: Updated the job-config configmap in namespace kubevirt-prow using the following files:

  • key kubernetes-nmstate-presubmits.yaml using file github/ci/prow/files/jobs/nmstate/kubernetes-nmstate-presubmits.yaml

In response to this:

We want to test against D/S too. The underlying system and the
NetworkManager version there may be different. Also, OCP lane seems
to be more stable than OKD.

This also mark the OKD lane as optional since it is consistently
not working.

Signed-off-by: Petr Horacek [email protected]

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.

gabrielecerami pushed a commit to gabrielecerami/project-infra that referenced this pull request Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants