Skip to content

Bug 1792005: lib/resourcemerge/core: Clear livenessProbe and readinessProbe if nil in required #300

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

Merged
openshift-merge-robot merged 1 commit into from
Feb 20, 2020
Merged

Bug 1792005: lib/resourcemerge/core: Clear livenessProbe and readinessProbe if nil in required #300

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
lib/resourcemerge/core: Clear livenessProbe and readinessProbe if nil… 
… in required

We claimed:

  if we have no required, then we don't care what someone else has set

since this code landed in d9f6718 (lib: add lib for applying
objects, 2018-08-14, #7).  But we do care in situations like [1],
where a 4.3 -> 4.2 downgrade leaves 4.3 readiness probes in place for
a 4.2 operator which has no /readyz.

With this commit we still have a few other types where we claim to not
care:

  $ git grep -hB1 'someone else has set' | grep func
  func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) {
  func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) {
  func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) {
  func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) {
  func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) {
  func setBoolPtr(modified *bool, existing **bool, required *bool) {
  func setInt64Ptr(modified *bool, existing **int64, required *int64) {

but I'm leaving them alone until we have a clearer picture about
whether we care about those specific properties or not.

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1791863#c1
  • Loading branch information
@wking
wking authored and openshift-cherrypick-robot committed Jan 17, 2020
commit e24b40ac3ce4c2042145a3356b4e31145b58d391
13 changes: 4 additions & 9 deletions lib/resourcemerge/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,8 @@ func ensureContainer(modified *bool, existing *corev1.Container, required corev1
ensureVolumeMount(modified, existingCurr, required)
}

if required.LivenessProbe != nil {
ensureProbePtr(modified, &existing.LivenessProbe, required.LivenessProbe)
}
if required.ReadinessProbe != nil {
ensureProbePtr(modified, &existing.ReadinessProbe, required.ReadinessProbe)
}
ensureProbePtr(modified, &existing.LivenessProbe, required.LivenessProbe)
ensureProbePtr(modified, &existing.ReadinessProbe, required.ReadinessProbe)

// our security context should always win
ensureSecurityContextPtr(modified, &existing.SecurityContext, required.SecurityContext)
Expand All @@ -170,11 +166,10 @@ func ensureEnvFromSource(modified *bool, existing *[]corev1.EnvFromSource, requi
}

func ensureProbePtr(modified *bool, existing **corev1.Probe, required *corev1.Probe) {
// if we have no required, then we don't care what someone else has set
if required == nil {
if *existing == nil && required == nil {
return
}
if *existing == nil {
if *existing == nil || required == nil {
*modified = true
*existing = required
return
Expand Down