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

prow: enable stage plugin for k/features and k/sig-release #8340

Merged
merged 1 commit into from
Jun 15, 2018

Conversation

nikhita
Copy link
Member

@nikhita nikhita commented Jun 12, 2018

Fixes #8315
Follow up to #8335

This enables the stage plugin for the k/features and k/sig-release repos.

/assign fejta cblecker

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 12, 2018
@nikhita nikhita mentioned this pull request Jun 12, 2018
@nikhita
Copy link
Member Author

nikhita commented Jun 12, 2018

I think the tests should pass once the stage plugin is added in #8335.

Adding an explicit hold until it is merged.
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 12, 2018
@nikhita
Copy link
Member Author

nikhita commented Jun 12, 2018

/area prow

@k8s-ci-robot k8s-ci-robot added the area/prow Issues or PRs related to prow label Jun 12, 2018
@nikhita nikhita changed the title prow: enable stage plugin for k/features prow: enable stage plugin for k/features and k/sig-release Jun 12, 2018
@@ -316,6 +317,7 @@ plugins:

kubernetes/sig-release:
- approve
- stage
Copy link
Member Author

Choose a reason for hiding this comment

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

added for sig-release as well, as per #8335 (comment)

@@ -45,6 +45,7 @@ import (
_ "k8s.io/test-infra/prow/plugins/size"
_ "k8s.io/test-infra/prow/plugins/skip"
_ "k8s.io/test-infra/prow/plugins/slackevents"
_ "k8s.io/test-infra/prow/plugins/stage"
Copy link
Contributor

Choose a reason for hiding this comment

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

This cannot be in the same PR as the plugins.yaml change. It belongs in the PR that creates the plugin.

After we merge the prow/hook/plugins.go change we will need to cut and deploy a new prow release, after which time we can make the plugins.yaml change to enable 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.

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

@fejta
Copy link
Contributor

fejta commented Jun 12, 2018

/lgtm
/hold

Please wait until a prow bump after #8335 merges

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 12, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fejta, nikhita

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 12, 2018
@fejta
Copy link
Contributor

fejta commented Jun 12, 2018

Hey! Looks like we test for this.

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //prow/cmd/hook:go_default_test
-----------------------------------------------------------------------------
--- FAIL: TestPlugins (0.03s)
	main_test.go:29: Could not load plugins: invalid plugin configuration:
			unknown plugin: stage
			unknown plugin: stage.
FAIL

If we were really clever we would make this read the hook tag in prow/cluster/hook_deployment.yaml, check out that test-infra commit and run the test there.

We can just wait till the other PR merges and /retest to fix

@justaugustus
Copy link
Member

/hold cancel
/retest

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2018
@nikhita
Copy link
Member Author

nikhita commented Jun 15, 2018

/hold

I think we still need to bump prow.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2018
@justaugustus
Copy link
Member

Hey @nikhita, I think based on the original test failure above, tests here would continue to fail if prow hadn't integrated the new stage plugin. Since we're all green, I'm going to pull the hold.

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 15, 2018
@k8s-ci-robot k8s-ci-robot merged commit 6616384 into kubernetes:master Jun 15, 2018
@k8s-ci-robot
Copy link
Contributor

@nikhita: Updated the plugins configmap from file prow/plugins.yaml

In response to this:

Fixes #8315
Follow up to #8335

This enables the stage plugin for the k/features and k/sig-release repos.

/assign fejta cblecker

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.

@nikhita
Copy link
Member Author

nikhita commented Jun 15, 2018

@nikhita
Copy link
Member Author

nikhita commented Jun 15, 2018

I think we still need a prow bump for this to work.

image: gcr.io/k8s-prow/hook:v20180608-68f2851db

@fejta
Copy link
Contributor

fejta commented Jun 15, 2018

Correct, we still need to bump prow

@fejta
Copy link
Contributor

fejta commented Jun 15, 2018

tests here would continue to fail if prow hadn't integrated the new stage plugin

@justaugustus unfortunately it just means the new stage plugin has merged into master. Doesn't necessarily mean a new prow has been deployed yet.

@justaugustus
Copy link
Member

@fejta I see. Is it a function of test-infra to bump prow or can a contributor submit a PR to do this as well?
There are instructions in the top-level of prow/, I'm just wondering if you need magic powers to use them.

@justaugustus
Copy link
Member

kubernetes/enhancements#571 (comment)

It's alive! Thanks for your help @nikhita & @fejta!

@fejta
Copy link
Contributor

fejta commented Jun 16, 2018

go.k8s.io/oncall has to deploy new versions of prow (which happened this afternoon)

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. area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants