Skip to content

add cluster roles to diagnostics #4702

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
openshift-bot merged 1 commit into from
Sep 17, 2015
Merged

add cluster roles to diagnostics #4702

openshift-bot merged 1 commit into from
Sep 17, 2015

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Sep 16, 2015

Adds a diagnostic test to see if the cluster-roles are up to date. It produces output like:

[Note] Running diagnostic: ClusterRoles
       Description: Check that the ClusterRoles are up-to-date

ERROR: [ClusterRolesDiagnostic1005 from diagnostic ClusterRoles@openshift/origin/pkg/diagnostics/cluster/roles.go:85]
       clusterrole/basic-user has changed and the existing role does not have enough permissions.

or

[Note] Running diagnostic: ClusterRoles
       Description: Check that the ClusterRoles are up-to-date

WARN:  [ClusterRolesDiagnostic1003 from diagnostic ClusterRoles@openshift/origin/pkg/diagnostics/cluster/roles.go:80]
       clusterrole/basic-user has changed, but the existing role covers the new role.

@sosiouxme ptal. Are recommendations simply embedded directly in the error message?

@sosiouxme
Copy link
Member

sosiouxme commented Sep 17, 2015

@deads2k yes, explanations and recommendations are just embedded in the error message. What would someone want to do if they encountered these?

sosiouxme
sosiouxme reviewed Sep 17, 2015
View reviewed changes
pkg/diagnostics/cluster/roles.go Outdated
Copy link
Member

@sosiouxme sosiouxme Sep 17, 2015

Choose a reason for hiding this comment

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

adjust godoc here

Copy link
Contributor Author

@deads2k deads2k Sep 17, 2015

Choose a reason for hiding this comment

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

adjust godoc here

done

sosiouxme
sosiouxme reviewed Sep 17, 2015
View reviewed changes
pkg/diagnostics/cluster/roles.go Outdated
Copy link
Member

@sosiouxme sosiouxme Sep 17, 2015

Choose a reason for hiding this comment

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

"is has"

Copy link
Contributor Author

@deads2k deads2k Sep 17, 2015

Choose a reason for hiding this comment

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

"is has"

fixed

@sosiouxme
Copy link
Member

sosiouxme commented Sep 17, 2015

Aside from the nit and the test failure, LGTM. Thanks!

@deads2k
Copy link
Contributor Author

deads2k commented Sep 17, 2015

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 17, 2015

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3345/) (Image: devenv-fedora_2361)

@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 17, 2015

Evaluated for origin merge up to a7a341d

openshift-bot pushed a commit that referenced this pull request Sep 17, 2015
Merge pull request #4702 from deads2k/roles-diagnostics
44265d3 
Merged by openshift-bot
@openshift-bot openshift-bot merged commit 44265d3 into openshift:master Sep 17, 2015
@deads2k deads2k deleted the roles-diagnostics branch November 5, 2015 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@deads2k @sosiouxme @openshift-bot