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

Charts[GKE-BUG] Apply nodeSelector to all pods in the chart including proxies #373

Closed
Tracked by #368
leninmehedy opened this issue Sep 22, 2023 · 8 comments · Fixed by #384
Closed
Tracked by #368

Charts[GKE-BUG] Apply nodeSelector to all pods in the chart including proxies #373

leninmehedy opened this issue Sep 22, 2023 · 8 comments · Fixed by #384
Assignees
Labels
Bug A error that causes the feature to behave differently than what was expected based on design docs released

Comments

@leninmehedy
Copy link
Member

leninmehedy commented Sep 22, 2023

Currently network-node pods are scheduled based on the following settings:

  nodeSelectors:
    fullstack-scheduling.io/os: linux
    fullstack-scheduling.io/role: network
  tolerations:
    - key: "fullstack-scheduling.io/os"
      operator: "Equal"
      value: "linux"
      effect: "NoSchedule"
    - key: "fullstack-scheduling.io/role"
      operator: "Equal"
      value: "network"
      effect: "NoSchedule"

We need to ensure these settings are applied for all pods in the chart including proxies.

@github-project-automation github-project-automation bot moved this to 🆕 New in FST Suite Sep 22, 2023
@leninmehedy leninmehedy added the Bug A error that causes the feature to behave differently than what was expected based on design docs label Sep 22, 2023
@leninmehedy leninmehedy self-assigned this Sep 22, 2023
@leninmehedy
Copy link
Member Author

leninmehedy commented Oct 5, 2023

It is tricky to pass nodeSelector and tolerations to dependent charts. After updating the values file, I was expecting all pods to be in pending state (since they should not have any appropriate node), however some of the pods from dependent charts are still running...

Screenshot 2023-10-05 at 1 22 10 PM

Besides, it makes our values.yaml file too much verbose since we need to duplicate and maintain nodeSelector and toleration settings for all dependent charts (see PR: #384).

@steven-sheehy
Copy link
Member

Can just disable rest-monitor via rest.monitor.enabled=false. It's not necessary for FST. Should also probably disable other monitor, rosetta, and graphql. We've disabled the latter two by default in latest RC.

@steven-sheehy
Copy link
Member

And with some config we can get rid of redis and pgpool, if we want to further reduce the number of containers.

@leninmehedy
Copy link
Member Author

Thanks @steven-sheehy

@leninmehedy
Copy link
Member Author

leninmehedy commented Oct 6, 2023

And with some config we can get rid of redis and pgpool, if we want to further reduce the number of containers.

@steven-sheehy Could you point me to the config? If redis and pgpool are not required, I can disable those by default in our chart.

Here is a list of containers after disabling some of the mirror-node containers as you suggested. Does that look ok?

Screenshot 2023-10-06 at 12 51 48 PM

@steven-sheehy
Copy link
Member

Could you point me to the config? If redis and pgpool are not required, I can disable those by default in our chart.

Sure, you can borrow from our GCP Marketplace values.yaml. Specifically, copy the
grpc.config, importer.config except cloudProvider, and postgresql.pgpool.replicaCount=0.

Here is a list of containers after disabling some of the mirror-node containers as you suggested. Does that look ok?

The list of containers is okay, but rest and importer pods are not ready. Do you know why?

@leninmehedy
Copy link
Member Author

The list of containers is okay, but rest and importer pods are not ready. Do you know why?

It just takes a bit longer to be ready.

Screenshot 2023-10-06 at 2 50 45 PM

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in FST Suite Oct 15, 2023
@swirlds-automation
Copy link
Contributor

🎉 This issue has been resolved in version 0.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A error that causes the feature to behave differently than what was expected based on design docs released
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants