search

knative / operator

Combined operator for Knative.
Apache License 2.0
179 stars 98 forks source link

Operator does not cleanly allow node drains to complete #1701

Open braunsonm opened 4 months ago

braunsonm commented 4 months ago

Describe the bug On AWS EKS, nodes are set to SchedulingDisabled and pods are evicted in batches (not cordoned). With knative serving deployed using the operator, some workloads will never drain when HA is set to 3.

Expected behavior The Knative Operator should allow these components to drain without user interaction.

To Reproduce

  1. Deploy Knative Serving on EKS with HA set to 3 using the Knative Operator
  2. Upgrade the version of Kubernetes by updating the AMI template. This will trigger AWS to do a rolling upgrade of the nodes
  3. Notice that knative-serving components cause the operation to hand indefinitely until a human forcibly kills the pod in question.

Knative release version 1.13.0

Additional context I have enough nodes that the PDB shouldn't be violated.

braunsonm commented 4 months ago

I found the problem. When HA is set to 3, the operator creates a PDB where minAvailable is set to 80%. This will never allow any of those pods to be evicted since 1 unavailable would be 66%.

houshengbo commented 4 months ago

@braunsonm Thanks for reporting the issue. Do you have any suggestion on how operator can change or improve to avoid this issue?

braunsonm commented 4 months ago

@houshengbo I think the operator should set maxUnavailable to 1 as a sensible default (roll each of these critical components one at a time). And continue to allow the user to override that.

houshengbo commented 4 months ago

Is maxUnavailable for knative serving or knative eventing? I would rather say this configuration should be for them, right? Operator does not by default configure them, instead, it read the manifests for them and use the default values from serving or eventing. You can use operator CRs to configure PodDisruptionBudget.

braunsonm commented 4 months ago

Yes and no. Max unavailable would be set for serving I think. But if you're configuring HA it would make sense that the operator creates a PDB so that HA is actually guaranteed. Otherwise you could still have an outage if the pods are evicted at the same time.

I agree allowing overrides though like you currently do.

github-actions[bot] commented 4 weeks ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

9numbernine9 commented 4 weeks ago

/remove-lifecycle stale