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

Add custom values file for scalardb-graphql Helm chart #180

Merged
merged 3 commits into from
Aug 6, 2022

Conversation

ymorimo
Copy link
Contributor

@ymorimo ymorimo commented Jul 1, 2022

This PR adds a custom values file for scalardb-graphql Helm chart.

This file is going to be referenced from the deployment guide. scalar-labs/scalardb-graphql#18

Currently, it covers AWS only. The database configurations and the Ingress annotations for other services should be added when needed.

@ymorimo ymorimo added the improvement updating the existing feature in a better way such as refactoring. label Jul 1, 2022
@ymorimo ymorimo self-assigned this Jul 1, 2022
Copy link
Contributor

@kota2and3kan kota2and3kan left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, but I have comments about tolerations and podAntiAffinity.


1. About tolerations

It seems that there is no configuration of tolerations like scalardl-custom-values.yaml and scalardb-custom-values.yaml.

https://github.com/scalar-labs/scalar-kubernetes/blob/master/conf/scalardl-custom-values.yaml#L99-L103
https://github.com/scalar-labs/scalar-kubernetes/blob/master/conf/scalardb-custom-values.yaml#L86-L90

I think some non Scalar DB GraphQL Server pods (e.g. user application pod) will be deployed to the node group for Scalar DB GraphQL Server, if there is no configuration of tolerations.
So, I think it is better to add tolerations to this sample custom values file.
(Also, in this case, we need to add the section that explain about adding Taints to the Kubernetes worker node in the deployment guide.)
What do you think?


2. About podAntiAffinity

It seems that there is no configuration of podAntiAffinity like scalardl-custom-values.yaml and scalardb-custom-values.yaml.
https://github.com/scalar-labs/scalar-kubernetes/blob/master/conf/scalardb-custom-values.yaml#L101-L115
https://github.com/scalar-labs/scalar-kubernetes/blob/master/conf/scalardl-custom-values.yaml#L114-L128

I think several Scalar DB GraphQL pods will be deployed to the same one node, if there is no configuration of podAntiAffinity.
So, I think it is better to add podAntiAffinity to distribute pods within the Kubernetes cluster.
What do you think?

@ymorimo
Copy link
Contributor Author

ymorimo commented Jul 7, 2022

@kota2and3kan Thank you for the suggestion. I didn't fully understand how tolerations and podAntiAffinity work, but now they are clearer thanks to your explanation.
I have added them in ff82496 and I'm going to add a description about Taints to the deployment guide right away.

@kota2and3kan
Copy link
Contributor

@ymorimo

I have added them in ff82496 and I'm going to add a description about Taints to the deployment guide right away.

Thank you for the update!
For your reference, I created the PR about Taints in #183.

@ymorimo
Copy link
Contributor Author

ymorimo commented Aug 2, 2022

@kota2and3kan Sorry for the late reply. I have added a description about Taints in scalar-labs/scalardb-graphql#32. Please take a look.

@ymorimo ymorimo requested a review from kota2and3kan August 2, 2022 04:26
Copy link
Contributor

@kota2and3kan kota2and3kan left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!


It worked well in my local environment!

  • tolerations

    $ kubectl describe node minikube | grep Taints
    Taints:             <none>
    
    $ kubectl describe node minikube-m02 | grep Taints
    Taints:             kubernetes.io/app=scalardbgraphqlpool:NoSchedule
    $ kubectl get pod -o wide
    NAME                                READY   STATUS    RESTARTS   AGE   IP           NODE           NOMINATED NODE   READINESS GATES
    mysql-scalardb-0                    1/1     Running   0          50m   172.17.0.4   minikube       <none>           <none>
    postgresql-scalardb-0               1/1     Running   0          50m   172.17.0.5   minikube       <none>           <none>
    scalardb-6644875756-28fld           1/1     Running   0          48m   172.17.0.7   minikube       <none>           <none>
    scalardb-envoy-84bf44bc5d-84c88     1/1     Running   0          48m   172.17.0.6   minikube       <none>           <none>
    scalardb-graphql-7db6cdd8b5-8596x   0/1     Running   0          2s    172.17.0.4   minikube-m02   <none>           <none>
    scalardb-graphql-7db6cdd8b5-vg9k5   0/1     Running   0          3s    172.17.0.3   minikube-m02   <none>           <none>
    scalardb-graphql-7db6cdd8b5-zls4b   0/1     Running   0          2s    172.17.0.2   minikube-m02   <none>           <none>
    • Scalar DB GraphQL pods run on minikube-m02 and other pods run on minikube. In other words, I was able to make minikube-m02 dedicated node for Scalar DB GraphQL.
  • podAntiAffinity (I use requiredDuringSchedulingIgnoredDuringExecution for testing.)

    $ kubectl get pod -o wide
    NAME                                READY   STATUS    RESTARTS   AGE   IP           NODE           NOMINATED NODE   READINESS GATES
    mysql-scalardb-0                    1/1     Running   0          52m   172.17.0.4   minikube       <none>           <none>
    postgresql-scalardb-0               1/1     Running   0          51m   172.17.0.5   minikube       <none>           <none>
    scalardb-6644875756-28fld           1/1     Running   0          49m   172.17.0.7   minikube       <none>           <none>
    scalardb-envoy-84bf44bc5d-84c88     1/1     Running   0          49m   172.17.0.6   minikube       <none>           <none>
    scalardb-graphql-7bf5c4744c-52gc4   0/1     Pending   0          2s    <none>       <none>         <none>           <none>
    scalardb-graphql-7bf5c4744c-qrcpv   0/1     Running   0          2s    172.17.0.2   minikube-m02   <none>           <none>
    scalardb-graphql-7bf5c4744c-xfmtt   0/1     Pending   0          2s    <none>       <none>         <none>           <none>
    • Only one Scalar DB GraphQL pod runs on one node.

Copy link
Collaborator

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

Left a minor suggestion.
Other than that, LGTM! Thank you!

Copy link

@brfrn169 brfrn169 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Co-authored-by: Hiroyuki Yamada <mogwaing@gmail.com>
@brfrn169 brfrn169 merged commit 916ec68 into master Aug 6, 2022
@brfrn169 brfrn169 deleted the scalardb-graphql-custom-values branch August 6, 2022 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement updating the existing feature in a better way such as refactoring.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants