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

Separate nodeSelectors for non vizier-pem pods #1861

Open
ashutoshrathore opened this issue Mar 26, 2024 · 2 comments
Open

Separate nodeSelectors for non vizier-pem pods #1861

ashutoshrathore opened this issue Mar 26, 2024 · 2 comments
Assignees

Comments

@ashutoshrathore
Copy link

Is your feature request related to a problem? Please describe.
Pixie deploys various pods like vizier-query broker, vizier cloud connector, kelvin and vizier-pem.

When we try to add nodeSelector for these pods using values defined here , it also selected the node pool for vizier-pem and that actually disable the function of vizier-pem to run on all nodes in cluster.

Pem already has pre-defined value for memory i.e. pemMemoryRequest and pemMemoryLimit

Describe the solution you'd like
nodeSelector should only work for pods that dont require to run on all nodes like query broker or kelvin.
Vizier-pem should have separate resource blocks. Rather if we can have separate defination for each deployments then it will help to customise in a better way for users.

Describe alternatives you've considered
No alternatives available

Additional context
NA

@ashutoshrathore
Copy link
Author

@aimichelle FYI

@ddelnano ddelnano self-assigned this May 3, 2024
ddelnano added a commit that referenced this issue May 9, 2024
#1887)

Summary: Exclude DaemonSets (PEMs) from modification by a Vizier's
nodeSelector

This accomplishes part of #1861. The remaining work is to include new
additional node selector configuration values that only apply to PEMs --
similar to how `pemMemoryLimit` and `pemMemoryRequest` work. From my
investigation so far, it appears that adding the PEM specific selectors
will require a different implementation (via the vizier yaml templating
on the Pixie cloud side) and so I thought staging this in two changes
made sense.

Relevant Issues: #1861

Type of change: /kind bug

Test Plan: Skaffolded a vizier operator and verified that setting a
`kubernetes.io/hostname` node selector no longer prevents PEMs from
being scheduled on all nodes

Changelog Message: Fixed an issue that caused the `Vizier` CRD to apply
node selectors to pods that should be scheduled on all nodes
(DaemonSets)

---------

Signed-off-by: Dom Delnano <ddelnano@gmail.com>
@ddelnano
Copy link
Member

@ashutoshrathore gave this a try once #1887 was merged. While this fixed the issues with the DaemonSet pods, the expected behaviors was to be able to control the scheduling of the OLM and operator pods as well. #1913 implements this by adding two new node selector configuration options in the helm chart.

ddelnano added a commit that referenced this issue May 24, 2024
…and operator pods (#1913)

Summary: Add node selectors fields in helm chart to control scheduling
of OLM and operator pods

Previously a helm chart user could only specify the node selectors for
the vizier components. This PR allows for applying node selectors to OLM
and the operator. This was feedback from @ashutoshrathore after the
functionality from #1887 was released.

Relevant Issues: #1861

Type of change: /kind feature

Test Plan: Tested the following scenarios
- [x] Used `helm template` to verify that the manifests are the same if
node selectors are not defined
```
$ helm template pixie-operator k8s/operator/helm/ -f k8s/operator/helm/values.yaml --debug  > new.yaml
install.go:200: [debug] Original chart version: ""
install.go:217: [debug] CHART PATH: /home/ddelnano/code/pixie-worktree/k8s/operator/helm

# Switch branches to capture templated output from existing helm chart
$ git checkout main
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
$ helm template pixie-operator k8s/operator/helm/ -f k8s/operator/helm/values.yaml --debug  > old.yaml
install.go:200: [debug] Original chart version: ""
install.go:217: [debug] CHART PATH: /home/ddelnano/code/pixie-worktree/k8s/operator/helm

$ diff old.yaml new.yaml
$
```
- [x] Deployed a local version of the helm chart to a cluster and
verified that the node selector values are set properly for pods in the
`olm` and `px-operator` namespace
- [x] Verified that pods failed to schedule if the pinned host was
terminated
```
$ kubectl -n olm get pods
NAME                                READY   STATUS    RESTARTS   AGE
catalog-operator-77554fbc46-zjq8m   0/1     Pending   0          2m13s
olm-operator-76dc499446-d8qvh       0/1     Pending   0          2m11s
$ kubectl -n px-operator get pods
NAME                               READY   STATUS    RESTARTS   AGE
vizier-operator-5ff9749c94-98q87   0/1     Pending   0          2m17s
```

Changelog Message: Add support for specifying node selector in the helm
chart for OLM and operator pods

Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants