Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

fix: add rbac rules for nodes/status in cloud-node-manager.yaml #3699

Merged
merged 2 commits into from
Aug 17, 2020

Conversation

nilo19
Copy link
Member

@nilo19 nilo19 commented Aug 14, 2020

Reason for Change:

Add rbac rules for nodes/status in cloud-node-manager.yaml.

Issue Fixed:

Requirements:

Notes:

@nilo19 nilo19 force-pushed the bug/add-nodes-status-rbac branch 2 times, most recently from e3c44b7 to d68e302 Compare August 14, 2020 02:32
feiskyer
feiskyer previously approved these changes Aug 14, 2020
Copy link
Member

@feiskyer feiskyer left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@mboersma
Copy link
Member

Add rbac rules for nodes/status in cloud-node-manager.yaml.

That describes what this PR does, but isn't a reason: why do we need to add this RBAC permission? It's not in the example manifest at kubernetes-sigs/cloud-provider-azure, so I'm just curious why the change is needed.

@nilo19
Copy link
Member Author

nilo19 commented Aug 14, 2020

example manifest

kubernetes-sigs/cloud-provider-azure#372 the PR is adding it in the example manifest. We need this because it blocks several of our tests in here.

@mboersma
Copy link
Member

/lgtm

@mboersma
Copy link
Member

/azp run pr-e2e

@nilo19 I rebased this off current master because CI testing will fail without #3707.

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@mboersma
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mboersma mboersma changed the title Add rbac rules for nodes/status in cloud-node-manager.yaml fix: add rbac rules for nodes/status in cloud-node-manager.yaml Aug 17, 2020
@mboersma
Copy link
Member

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov
Copy link

codecov bot commented Aug 17, 2020

Codecov Report

Merging #3699 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3699   +/-   ##
=======================================
  Coverage   73.16%   73.16%           
=======================================
  Files         147      147           
  Lines       25318    25318           
=======================================
  Hits        18525    18525           
  Misses       5655     5655           
  Partials     1138     1138           
Impacted Files Coverage Δ
pkg/engine/templates_generated.go 53.42% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a83dff8...1ba069d. Read the comment docs.

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot
Copy link

acs-bot commented Aug 17, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, feiskyer, mboersma, nilo19

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:
  • OWNERS [CecileRobertMichon,mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mboersma mboersma merged commit f78f049 into Azure:master Aug 17, 2020
@nilo19 nilo19 deleted the bug/add-nodes-status-rbac branch August 18, 2020 00:57
penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants