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

fix: ignore container ordering for pod diff #499

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
### Fixed Bugs

- [PR #476](https://github.com/konpyutaika/nifikop/pull/476) - **[Operator/NifiCluster]** Fixed istio init race condition and pod reconciliation.
- [PR #499](https://github.com/konpyutaika/nifikop/pull/499) - **[Operator/NifiCluster]** Fixed istio container ordering pod reconciliation.

### Deprecated

Expand Down
66 changes: 56 additions & 10 deletions pkg/resources/nifi/nifi.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package nifi

import (
"context"
"encoding/json"
"fmt"
"reflect"
"sort"
Expand Down Expand Up @@ -691,7 +692,6 @@ func (r *Reconciler) reconcileNifiPod(log zap.Logger, desiredPod *corev1.Pod) (e
uniqueContainers = append(uniqueContainers, c)
}
}
desiredPod.Spec.InitContainers = uniqueContainers
}
// If there are extra containers from webhook injections we need to add them
if len(currentPod.Spec.Containers) > len(desiredPod.Spec.Containers) {
Expand All @@ -704,29 +704,29 @@ func (r *Reconciler) reconcileNifiPod(log zap.Logger, desiredPod *corev1.Pod) (e
uniqueContainers = append(uniqueContainers, c)
}
}
desiredPod.Spec.Containers = uniqueContainers
}
// Remove problematic fields if istio
if _, ok := currentPod.Annotations["istio.io/rev"]; ok {
// Prometheus scrape port is overridden by istio injection
delete(currentPod.Annotations, "prometheus.io/port")
delete(desiredPod.Annotations, "prometheus.io/port")
// Liveness probe port is overridden by istio injection
desiredContainer := corev1.Container{}
for _, c := range desiredPod.Spec.Containers {
currentContainer := corev1.Container{}
for _, c := range currentPod.Spec.Containers {
if c.Name == "nifi" {
desiredContainer = c
currentContainer = c
}
}
currentContainers := []corev1.Container{}
for _, c := range currentPod.Spec.Containers {
desiredContainers := []corev1.Container{}
for _, c := range desiredPod.Spec.Containers {
if c.Name == "nifi" {
c.LivenessProbe = desiredContainer.LivenessProbe
c.LivenessProbe = currentContainer.LivenessProbe
}
currentContainers = append(currentContainers, c)
desiredContainers = append(desiredContainers, c)
}
currentPod.Spec.Containers = currentContainers
desiredPod.Spec.Containers = desiredContainers
}

// Check if the resource actually updated
patchResult, err := patch.DefaultPatchMaker.Calculate(currentPod, desiredPod, opts...)
if err != nil {
Expand Down Expand Up @@ -755,6 +755,52 @@ func (r *Reconciler) reconcileNifiPod(log zap.Logger, desiredPod *corev1.Pod) (e
return nil, k8sutil.PodReady(currentPod)
}
} else {
// We want to check if the only part of the patch is element ordering, in which case we ignore it due to injection
var patch map[string]interface{}
if err := json.Unmarshal(patchResult.Patch, &patch); err == nil {
ignoreSpec := false
if len(patch) == 1 {
if spec, ok := patch["spec"].(map[string]interface{}); ok {
// If we only have one item in the spec then we check for container or initContainer ordering
if len(spec) == 1 {
if _, ok := spec["$setElementOrder/containers"]; ok {
ignoreSpec = true
} else if _, ok := spec["$setElementOrder/initContainers"]; ok {
ignoreSpec = true
}
// If we have two items in the spec then we check for container and initContainer ordering
} else if len(spec) == 2 {
if _, ok := spec["$setElementOrder/containers"]; ok {
if _, ok := spec["$setElementOrder/initContainers"]; ok {
ignoreSpec = true
}
}
}
}
}
// Since we can ignore the spec diff due to it only including $setElementOrder we'll proceed as if the pod is in sync
if ignoreSpec {
if !k8sutil.IsPodTerminatedOrShutdown(currentPod) &&
r.NifiCluster.Status.NodesState[currentPod.Labels["nodeId"]].ConfigurationState == v1.ConfigInSync {
if val, found := r.NifiCluster.Status.NodesState[desiredPod.Labels["nodeId"]]; found &&
val.GracefulActionState.State == v1.GracefulUpscaleRunning &&
val.GracefulActionState.ActionStep == v1.ConnectStatus &&
k8sutil.PodReady(currentPod) {
if err := k8sutil.UpdateNodeStatus(r.Client, []string{desiredPod.Labels["nodeId"]}, r.NifiCluster, r.NifiClusterCurrentStatus,
v1.GracefulActionState{ErrorMessage: "", State: v1.GracefulUpscaleSucceeded}, log); err != nil {
return errorfactory.New(errorfactory.StatusUpdateError{},
err, "could not update node graceful action state"), false
}
}

log.Debug("pod resource is in sync",
zap.String("clusterName", r.NifiCluster.Name),
zap.String("podName", currentPod.Name))

return nil, k8sutil.PodReady(currentPod)
}
}
}
log.Debug("resource diffs",
zap.String("patch", string(patchResult.Patch)),
zap.String("current", string(patchResult.Current)),
Expand Down