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

SKIP-478 Legge til deployment strategy og startup probe #20

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

eliihen
Copy link
Member

@eliihen eliihen commented Jul 25, 2022

Lagt til et par nye felter for å løse SKIP-478.

Endret også accessPolicy.external.rules til å være optional siden den ikke skal være påkrevet, og her oppdaget jeg også en annen feil (#19).

La også merke til at vi har endret fra cpuThresholdPercentage til targetCpuUtilization i refaktoreringen, så jeg oppdaterte doken til å gjenspeile det. Strengt tatt er det en breaking change som vi bør være flinke på å deprecate lenge før, men siden bare NRL bruker skiperator akkurat nå kan jeg kjøpe et argument om at vi kan være litt mer agressive på brekkinga akkurat nå som omfanget blir så lite.

@eliihen eliihen requested a review from hagen93 July 25, 2022 09:16
@hagen93
Copy link
Contributor

hagen93 commented Jul 25, 2022

Enig at vi bør være mer forsiktig med breaking changes. Var som du sa siden vi ikke er live enda så tok jeg litt lettere på det.

// +kubebuilder:default=RollingUpdate
Type string `json:"type,omitempty"`
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Noen grunn til å introdusere en egen struct bare for denne stringen? Har vi planer om mer rollout strategy options down the line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Tenkte det ja, begynte med dette nå så NRL kan få redeploya databasen sin så kan vi legge inn options for RollingUpdate og slikt her etter hvert

Copy link
Contributor

@hagen93 hagen93 left a comment

Choose a reason for hiding this comment

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

LGTM

@eliihen eliihen merged commit 5f143fa into main Jul 25, 2022
@eliihen eliihen deleted the startup-and-strategy branch July 25, 2022 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants