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

Remove RBAC console template #7120

Merged

Conversation

spadgett
Copy link
Member

The console is changing to discover if the template service broker is
running by looking at the service classes on the client. This means the
console no longer needs the RBAC template. It also no longer needs to
manually trigger a rollout after the template service broker is
installed or removed.

Requires:

openshift/origin-web-console-server#31
openshift/origin-web-console#2800
openshift/origin-web-catalog#642

Corresponding origin PR:

openshift/origin#18580

This will fail tests until the origin-web-console image is updated with the changes in the origin-web-console-server PR.

/assign @sdodson
@deads2k cc

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 12, 2018
@spadgett
Copy link
Member Author

/hold

@spadgett
Copy link
Member Author

The origin-web-console-server and origin changes have merged. @sdodson PTAL

/retest
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2018
@spadgett spadgett force-pushed the remove-console-rbac branch from 117f75e to ceb41f0 Compare February 20, 2018 14:11
@spadgett
Copy link
Member Author

/retest

@sdodson
Copy link
Member

sdodson commented Feb 20, 2018

/lgtm
Is there any clean up to do in environments that had the old code applied?

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 20, 2018
@sdodson
Copy link
Member

sdodson commented Feb 20, 2018

/test containerized

@sdodson
Copy link
Member

sdodson commented Feb 20, 2018

Not obvious to me why the console failed in that job, seems like it may've been crashlooping.

@spadgett
Copy link
Member Author

Is there any clean up to do in environments that had the old code applied?

Ideally you would remove the role binding and cluster role we were previously creating, although they won't break anything if they're there. It just gives the console a little more access than it needs (read permission on cluster service brokers).

Not obvious to me why the console failed in that job, seems like it may've been crashlooping.

Yeah, me either. Is it possibly using an old origin-web-console image?

#7132 would help :/

@spadgett spadgett force-pushed the remove-console-rbac branch from ceb41f0 to 49dfe62 Compare February 21, 2018 01:35
@spadgett
Copy link
Member Author

Rebased to pick up #7132 for debugging the test failures

@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2018
@spadgett
Copy link
Member Author

@sdodson The containerized tests are failing because they're using an old image: docker.io/openshift/origin-web-console:v3.9.0-alpha.4

@sdodson
Copy link
Member

sdodson commented Feb 21, 2018

Ok, so CI is setting the image tag for containerized installs, it'd be fixed if we overrided to 'latest' tag in CI or we tag a new image. @smarterclayton do you know when we'll next tag next?

@spadgett
Copy link
Member Author

On the bright side, what we log now on console failures is extremely useful

@spadgett
Copy link
Member Author

@sdodson We've switching to pushing to a v3.9 tag, so I don't think there will be any more v3.9.0-alpha tags

@spadgett
Copy link
Member Author

@sdodson Following up on this... It seems to me the tests should be using the v3.9 tag if possible.

@sdodson
Copy link
Member

sdodson commented Feb 27, 2018

/retest

@sdodson
Copy link
Member

sdodson commented Feb 27, 2018

@spadgett Got the CI changes that force image tag of 'latest' on master branch PR CI jobs, can you rebase?

@sdodson
Copy link
Member

sdodson commented Feb 27, 2018

The console is changing to discover if the template service broker is
running by looking at the service classes on the client. This means the
console no longer needs the RBAC template. It also no longer needs to
manually trigger a rollout after the template service broker is
installed or removed.
@spadgett spadgett force-pushed the remove-console-rbac branch from 49dfe62 to 4559402 Compare February 27, 2018 19:39
@spadgett
Copy link
Member Author

@sdodson thanks, rebased

@spadgett
Copy link
Member Author

/retest

@sdodson
Copy link
Member

sdodson commented Feb 28, 2018

/test containerized

@spadgett
Copy link
Member Author

/retest

@spadgett
Copy link
Member Author

/cherrypick release-3.9

@openshift-cherrypick-robot

@spadgett: once the present PR merges, I will cherry-pick it on top of release-3.9 in a new PR and assign it to you.

In response to this:

/cherrypick release-3.9

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.

@sdodson
Copy link
Member

sdodson commented Feb 28, 2018

/retest

1 similar comment
@spadgett
Copy link
Member Author

/retest

@spadgett
Copy link
Member Author

spadgett commented Mar 1, 2018

@sdodson Can you tag this for merge again? It lost the tag when I rebased.

@sdodson
Copy link
Member

sdodson commented Mar 1, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2018
@openshift-merge-robot
Copy link
Contributor

/test all [submit-queue is verifying that this PR is safe to merge]

@spadgett
Copy link
Member Author

spadgett commented Mar 1, 2018

/retest

@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 1, 2018

@spadgett: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/extended_conformance_install_crio 4559402 link /test crio
ci/openshift-jenkins/system-containers 4559402 link /test system-containers

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue.

@openshift-cherrypick-robot

@spadgett: new pull request created: #7346

In response to this:

/cherrypick release-3.9

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
Labels
lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants