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.

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

upgrade gatekeeper to version 3.11.1 #2954

Merged
merged 1 commit into from
Jul 19, 2023
Merged

upgrade gatekeeper to version 3.11.1 #2954

merged 1 commit into from
Jul 19, 2023

Conversation

yjst2012
Copy link
Contributor

Which issue this PR addresses:

Fixes ARO-3058

What this PR does / why we need it:

upgrade gatekeeper version from current 3.10.0 to 3.11.1 which is the latest stable

Test plan for issue:

tested on my local dev env

Is there any documentation that needs to be updated for this PR?

NA

@yjst2012 yjst2012 added skippy pull requests raised by member of Team Skippy ready-for-review next-release To be included in the next RP release rollout labels Jun 15, 2023
SudoBrendan
SudoBrendan reviewed Jul 11, 2023
View reviewed changes
Copy link
Collaborator

@SudoBrendan SudoBrendan left a comment

Choose a reason for hiding this comment

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

I'm a little disconnected from GK, so I left a few questions - nothing here that is very concerning on it's face, I'm just not really sure what I'm looking at to be honest :p

pkg/operator/controllers/guardrails/guardrails_config.go
@@ -41,11 +41,11 @@ const (

defaultManagerRequestsCPU = "100m"
defaultManagerLimitCPU = "1000m"
defaultManagerRequestsMem = "256Mi"
defaultManagerRequestsMem = "512Mi"
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity - is this following a recommendation per the version bump, or something unrelated?

Copy link
Member

Choose a reason for hiding this comment

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

Hi @yjst2012 I am also curious about this. Because it will have impact on a scheduler and the masters are of a fixed size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, this change is made by gatekeeper. well basically all stuff here is porting from upstream.

pkg/operator/controllers/guardrails/staticresources/crd_gk_config_configs.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is everything under staticresources auto-generated or is this something that needs thorough review?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it is a manual step, ie, compare the changed made to the upstream gatekeeper.yaml and move it to our corresponding files under staticresources/ if it is proper to us.

petrkotas
petrkotas reviewed Jul 18, 2023
View reviewed changes
Copy link
Member

@petrkotas petrkotas left a comment

Choose a reason for hiding this comment

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

Hi @yjst2012 it seems straightforward thanks! Just one question before we can get in in.

pkg/operator/controllers/guardrails/guardrails_config.go
@@ -41,11 +41,11 @@ const (

defaultManagerRequestsCPU = "100m"
defaultManagerLimitCPU = "1000m"
defaultManagerRequestsMem = "256Mi"
defaultManagerRequestsMem = "512Mi"
Copy link
Member

Choose a reason for hiding this comment

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

Hi @yjst2012 I am also curious about this. Because it will have impact on a scheduler and the masters are of a fixed size.

@yjst2012 yjst2012 merged commit 220c6ff into Azure:master Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrkotas petrkotas petrkotas approved these changes

@jewzaam jewzaam Awaiting requested review from jewzaam jewzaam is a code owner

@bennerv bennerv Awaiting requested review from bennerv bennerv is a code owner

@hawkowl hawkowl Awaiting requested review from hawkowl hawkowl is a code owner

@rogbas rogbas Awaiting requested review from rogbas rogbas is a code owner

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@cblecker cblecker Awaiting requested review from cblecker cblecker is a code owner

@facchettos facchettos Awaiting requested review from facchettos

@cadenmarchese cadenmarchese Awaiting requested review from cadenmarchese cadenmarchese is a code owner

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@s-amann s-amann Awaiting requested review from s-amann

@ellis-johnson ellis-johnson Awaiting requested review from ellis-johnson

@SudoBrendan SudoBrendan Awaiting requested review from SudoBrendan SudoBrendan is a code owner

Assignees
No one assigned
Labels
next-release To be included in the next RP release rollout ready-for-review skippy pull requests raised by member of Team Skippy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yjst2012 @petrkotas @SudoBrendan