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

node assignment configs are attached on all k8s workloads #1853

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

ranvit
Copy link
Contributor

@ranvit ranvit commented May 10, 2024

SUMMARY

The db migration pod and the backup process pod have failed to schedule on a k8s cluster where every node has a taint. So this PR adds the task_* related node-assignment params and the global node-assignment params as fallback to every k8s workload that didn't have node-assignment configuration available on them.

This fixes issues like #1774

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
ADDITIONAL INFORMATION

I PR'd a similar fix with #1804, only targeting the migration pod. Realised I need somethign similar on the backup pod.

The task_* params seem well suited for most of the k8s workloads except for the mesh-ingress deployment, so I had that only use the global params

@@ -20,9 +20,33 @@ spec:
resources:
{{ backup_resource_requirements | to_nice_yaml(indent=2) | indent(width=6, first=False) }}
{%- endif %}
{% if db_management_pod_node_selector %}
{% if task_node_selector %}
Copy link
Member

Choose a reason for hiding this comment

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

@ranvit The task_node_selector parameter only exists in the installer role (AWX CR), not in the back role (AWXBackup CR).

So db_management_pod_node_selector is correct.

If you need topologySpreadConstraints, tolerations, and affinity as well, those params would need to be added to the AWXBackup CR and should be prefixed with db_management_pod_*

Copy link
Member

Choose a reason for hiding this comment

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

This PR also needs a rebase now to resolve conflicts.

@ranvit ranvit mentioned this pull request May 28, 2024
3 tasks
@djyasin
Copy link
Member

djyasin commented Jul 24, 2024

Hello @ranvit, please let us know if you are still interested in merging this PR. If so, please be sure to address the comments left by @rooftopcellist and resolve any merge conflicts.

Thank you for your time!

Copy link

sonarcloud bot commented Jul 25, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants