-
-
Notifications
You must be signed in to change notification settings - Fork 5
Feature: Allow workloadKind: Deployment #873
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
Open
Maleware
wants to merge
15
commits into
main
Choose a base branch
from
feat/opa-as-deployment
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
d499dfa
feat: Add workloadKind, internalTrafficPolicy and PDB config to serve…
Maleware 8df4ece
Remove internalTrafficPolicy for now, waiting on the decision
Maleware 38d0c17
refactor before adding deployment.rs as parallel mode
Maleware 20e3081
Adding deployment.rs module to handle deployments of opa
Maleware 0ace30e
Adding PDBs if deployment is chosen
Maleware 61b8c20
Adding one test for opa deployment switch and PDBs
Maleware 08c2956
Merge branch 'main' into feat/opa-as-deployment
Maleware 595a6d4
Merge branch 'main' into feat/opa-as-deployment
Maleware ae908ec
Merge branch 'main' into feat/opa-as-deployment
Maleware 76fab10
Merge branch 'main' into feat/opa-as-deployment
Maleware d16fa00
Update rust/operator-binary/src/controller/build/resource/service.rs
Maleware f34677e
Update rust/operator-binary/src/controller/build/resource/service.rs
Maleware 55dd7af
Apply review comment
Maleware 5e7d2ef
Merge branch 'feat/opa-as-deployment' of github.com:stackabletech/opa…
Maleware da77bf7
Merging main into feat/opa-as-deployment
Maleware File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 37 additions & 3 deletions
40
docs/modules/opa/pages/usage-guide/operations/pod-disruptions.adoc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,39 @@ | ||
| = Allowed Pod disruptions | ||
| :description: Whether the operator creates a PodDisruptionBudget for OPA depends on the workload kind, and how to configure or disable that budget. | ||
|
|
||
| For OPA clusters, the operator does not deploy any PodDisruptionBudgets (PDBs), as there is one instance per Kubernetes node running (Daemonset). | ||
| When a Kubernetes node gets drained to gracefully shut it down, the OPA Pod get's evicted - there is no point in blocking the eviction. | ||
| In case the OPA Pod terminated before the products depending on OPA (e.g. Trino coordinator) on the same node, the products can still use the OPA Service, as it routes to OPA Pods running on other Kubernetes nodes. | ||
| You can configure the allowed Pod disruptions as described in xref:concepts:operations/pod_disruptions.adoc[]. | ||
|
|
||
| What the operator creates depends on the xref:usage-guide/workload-kind.adoc[workload kind] of the role. | ||
|
|
||
| == DaemonSet | ||
|
|
||
| No PodDisruptionBudget is created, and you should not enable one. | ||
|
|
||
| `kubectl drain` skips DaemonSet Pods, a budget is never consulted during a node drain. | ||
| Moreover, PodDisruptionBudgets over DaemonSets can never be evaluated: DaemonSet does not implement the scale subresource. | ||
| Therefore the budget stays at `disruptionsAllowed: 0` with a `SyncFailed` condition and refuses every direct eviction. | ||
|
|
||
| Because the role Service routes node-locally in this mode, products on a drained node cannot reach OPA on another node. | ||
| Drain nodes together with the products that query OPA on them. | ||
|
|
||
| == Deployment | ||
|
|
||
| The operator creates one PodDisruptionBudget per role, allowing one Pod to be unavailable at a time by default: | ||
|
|
||
| [source,yaml] | ||
| ---- | ||
| spec: | ||
| servers: | ||
| roleConfig: | ||
| podDisruptionBudget: | ||
| enabled: true # <1> | ||
| maxUnavailable: 1 # <2> | ||
| ---- | ||
| <1> Defaults to true for a Deployment. Set it to false to create no budget. | ||
| <2> Defaults to 1. Raise it to allow more Pods to be unavailable at once. | ||
|
|
||
| Keep `maxUnavailable` at 1 unless you have measured that OPA tolerates more. | ||
| Products query OPA on every request, budgets that drain too many Pods at once turn a node rotation into a platform-wide slowdown. | ||
|
|
||
| NOTE: A budget only helps when there is another Pod to fall back to. | ||
| With `replicas` set to 1, the single Pod can still be evicted. |
33 changes: 33 additions & 0 deletions
33
docs/modules/opa/pages/usage-guide/operations/pod-placement.adoc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| = Pod placement | ||
| :description: The default affinity spreads OPA Pods across Kubernetes nodes, which matters only for a Deployment. | ||
|
|
||
| You can configure Pod placement for OPA Pods as described in xref:concepts:operations/pod_placement.adoc[]. | ||
|
|
||
| == Defaults | ||
|
|
||
| The default affinity created by the operator is: | ||
|
|
||
| * Distribute all Pods of the `servers` role across nodes, so that multiple Pods don't end up on the same Kubernetes node (weight 70) | ||
|
|
||
| This constrains nothing for a DaemonSet, which already places exactly one Pod per node. | ||
| It matters for a Deployment, where several replicas would otherwise be free to share the same node. | ||
| See xref:usage-guide/workload-kind.adoc[]. | ||
|
|
||
| [source,yaml] | ||
| ---- | ||
| affinity: | ||
| podAntiAffinity: | ||
| preferredDuringSchedulingIgnoredDuringExecution: | ||
| - podAffinityTerm: | ||
| labelSelector: | ||
| matchLabels: | ||
| app.kubernetes.io/component: server | ||
| app.kubernetes.io/instance: cluster-name | ||
| app.kubernetes.io/name: opa | ||
| topologyKey: kubernetes.io/hostname | ||
| weight: 70 | ||
| ---- | ||
|
|
||
| NOTE: The default affinity is only preferred and not enforced, because not every setup has multiple Kubernetes nodes. | ||
| To enforce it, set your own `requiredDuringSchedulingIgnoredDuringExecution` affinity. | ||
| Be aware that a Deployment with more replicas than nodes then leaves Pods unschedulable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| = Workload kind | ||
| :description: Run the OPA servers as a DaemonSet or as a Deployment, and learn how that choice affects the Pod count, Service routing and changing it later. | ||
|
|
||
| By default the `servers` role runs as a DaemonSet, which places one OPA Pod on every Kubernetes node. | ||
| Set `workloadKind` to Deployment to run a fixed number of Pods instead. | ||
|
|
||
| [source,yaml] | ||
| ---- | ||
| spec: | ||
| servers: | ||
| roleConfig: | ||
| workloadKind: Deployment # <1> | ||
| roleGroups: | ||
| default: | ||
| replicas: 3 # <2> | ||
| ---- | ||
| <1> Either DaemonSet (default) or Deployment. | ||
| <2> Only used by Deployment, DaemonSet derives Pod count from the number of nodes. | ||
|
|
||
| The workload kind also decides whether the operator creates a xref:usage-guide/operations/pod-disruptions.adoc[PodDisruptionBudget], and it changes what the default xref:usage-guide/operations/pod-placement.adoc[Pod placement] achieves. | ||
|
|
||
| == Choosing a workload kind | ||
|
|
||
| Use a DaemonSet when every node runs products that query OPA. | ||
| Each product then queries the OPA Pod on its own node, so no policy query crosses the network. | ||
| The number of OPA Pods grows and shrinks with the node count. | ||
|
|
||
| Use a Deployment when the number of OPA Pods should be fixed. | ||
| You set the count with `replicas` and queries are spread across all Pods. | ||
| This fits large clusters, and clusters where only a few nodes run products that query OPA. | ||
|
|
||
| == Service routing | ||
|
|
||
| The operator derives the role Service's `internalTrafficPolicy` from the workload kind. | ||
|
|
||
| * A DaemonSet gets Local, so a query only reaches the OPA Pod on the client's own node. | ||
| This avoids the network hop, and a DaemonSet covers every node, thus such a Pod always exists. | ||
| * A Deployment gets Cluster, so a query reaches any OPA Pod of the role. | ||
| A Deployment's Pods do not cover every node necessarily, so node-local routing would leave products on the remaining nodes unable to reach OPA at all. | ||
|
|
||
| If the derived value doesn't suit your cluster, you can override it as described in <<override-traffic-policy>>. | ||
|
|
||
| [#override-traffic-policy] | ||
| == Override traffic policy | ||
|
|
||
| In edge cases (e.g. node autoscaling under load), it is useful to use a DaemonSet with `internalTrafficPolicy: Cluster`. | ||
| This can be achieved using xref:concepts:overrides.adoc#object-overrides[object overrides]: | ||
|
|
||
| [source,yaml] | ||
| ---- | ||
| apiVersion: opa.stackable.tech/v1alpha2 | ||
| kind: OpaCluster | ||
| metadata: | ||
| name: simple-opa | ||
| namespace: default | ||
| spec: | ||
| image: | ||
| productVersion: 1.16.2 | ||
| objectOverrides: | ||
| - apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: simple-opa-server | ||
| namespace: default | ||
| spec: | ||
| internalTrafficPolicy: Cluster # <1> | ||
| servers: | ||
| roleGroups: | ||
| default: {} | ||
| ---- | ||
| <1> Changes `internalTrafficPolicy` from Local to Cluster. | ||
|
|
||
| == Changing the workload kind | ||
|
|
||
| Changing `workloadKind` replaces the workload object, so policy queries can fail while the new Pods start up. | ||
| Products usually treat a failed policy query as a denied request. | ||
|
|
||
| Changing to DaemonSet is the more disruptive direction. | ||
| The role Service narrows to Local as soon as you apply the change, while the running Pods still belong to the outgoing Deployment and cover only some nodes. | ||
| Products on the remaining nodes fail until the DaemonSet has rolled out everywhere. | ||
|
|
||
| To ease the interruption, pin the traffic policy to Cluster as described in <<override-traffic-policy>>, and remove the override once the rollout has finished. | ||
| This keeps every product able to reach any OPA Pod throughout the change. | ||
| A short window in which no Pod is ready can still occur, because the outgoing workload is removed as the new one starts. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could deploy both workloads in parallel and delete the old workload + change the service only once the new workload is available to avoid disruption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea a lot and gave it some thought during implementation. I didn't do it for following reasons.
First, realising this would lead to a steep increase of resources used on a cluster. This of course depends on the replica and node count respectively, but can lead to a state where a
DeploymentorDeamonSetcannot be scheduled fully which leaves us somehow stuck half way in the switch.Which leads directly to the second point: What is the exact definition of the "new" cluster being ready enough? Ports open? Pods being served? Status: ready? I wasn't able to come up with something decent to guarantee correct availability.
Last, building this would need at least some sort of a state machine. We would somehow need to persist a "mid-migration" marker (could be an annotation for example) which needs to be placed and removed according to the newly deployed clusters state, which is kinda complex and needs some code to orchestrate that.
All in all it was for me to much effort for a result which has a difficult to foresee and recover failure mode.
This said, if someone has a better idea how this can be achieved easily, I'd really like to have it.