Skip to content

Conversation

@r-t-m
Copy link
Contributor

@r-t-m r-t-m commented Nov 13, 2023

Adds possibility to deploy resources in the namespace different from the release one, useful in scenarios with umbrella helm charts.

@CLAassistant
Copy link

CLAassistant commented Nov 13, 2023

CLA assistant check
All committers have signed the CLA.

@kevinmingtarja
Copy link
Contributor

kevinmingtarja commented Nov 29, 2023

Hi @r-t-m, thanks for the PR! I wanted to know more about the use case for this and the current limitations before reviewing.

So my understanding is that this change is useful if you are deploying the dgraph chart as part of a parent/umbrella chart, but you want the dgraph chart to be in a different namespace than the parent, and right now that isn't possible?

@kevinmingtarja
Copy link
Contributor

And also, have you tested these changes?

If yes, is it possible to post some kubectl get ... outputs as well to demonstrate how it looks with your changes.

@r-t-m
Copy link
Contributor Author

r-t-m commented Nov 30, 2023

So my understanding is that this change is useful if you are deploying the dgraph chart as part of a parent/umbrella chart, but you want the dgraph chart to be in a different namespace than the parent, and right now that isn't possible?

That's correct, it's not possible with helm deployment unless these changes would be merged.
Yes, I've tested the changes and they are nonbreaking ones.

# Deployment in default namespace, nothing new if deployed from main
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ helm install test1 ./charts/dgraph
NAME: test1
LAST DEPLOYED: Thu Nov 30 20:59:56 2023
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
1. You have just deployed Dgraph, version 'v23.0.1'.

   For further information:
     * Documentation: https://dgraph.io/docs/
     * Community and Issues: https://discuss.dgraph.io/
   
2. Get the Dgraph Alpha HTTP/S endpoint by running these commands.

    export ALPHA_POD_NAME=$(kubectl get pods --namespace default --selector "statefulset.kubernetes.io/pod-name=test1-dgraph-alpha-0,release=test1" --output jsonpath="{.items[0].metadata.name}")
    echo "Access Alpha HTTP/S using http://localhost:8080"
    kubectl --namespace default port-forward $ALPHA_POD_NAME 8080:8080

   NOTE: Change "http://" to "https://" if TLS was added to the Ingress, Load Balancer, or Dgraph Alpha service.
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ kubectl get all -n default
NAME                       READY   STATUS    RESTARTS   AGE
pod/test1-dgraph-alpha-0   1/1     Running   0          3m50s
pod/test1-dgraph-alpha-1   1/1     Running   0          3m50s
pod/test1-dgraph-alpha-2   1/1     Running   0          3m50s
pod/test1-dgraph-zero-0    1/1     Running   0          3m50s
pod/test1-dgraph-zero-1    1/1     Running   0          3m50s
pod/test1-dgraph-zero-2    1/1     Running   0          3m50s

NAME                                  TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)             AGE
service/kubernetes                    ClusterIP   10.96.0.1      <none>        443/TCP             4m51s
service/test1-dgraph-alpha            ClusterIP   10.96.77.140   <none>        8080/TCP,9080/TCP   3m51s
service/test1-dgraph-alpha-headless   ClusterIP   None           <none>        7080/TCP            3m51s
service/test1-dgraph-zero             ClusterIP   10.96.191.20   <none>        5080/TCP,6080/TCP   3m51s
service/test1-dgraph-zero-headless    ClusterIP   None           <none>        5080/TCP            3m51s

NAME                                  READY   AGE
statefulset.apps/test1-dgraph-alpha   3/3     3m50s
statefulset.apps/test1-dgraph-zero    3/3     3m50s
# Deployment in helmrelease namespace with target namespace set to test
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ kubectl create namespace helmrelease
namespace/helmrelease created
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ kubectl create namespace test
namespace/test created
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ helm install test1 ./charts/dgraph --namespace helmrelease --set namespaceOverride=test
NAME: test1
LAST DEPLOYED: Thu Nov 30 21:09:24 2023
NAMESPACE: helmrelease
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
1. You have just deployed Dgraph, version 'v23.0.1'.

   For further information:
     * Documentation: https://dgraph.io/docs/
     * Community and Issues: https://discuss.dgraph.io/
   
2. Get the Dgraph Alpha HTTP/S endpoint by running these commands.

    export ALPHA_POD_NAME=$(kubectl get pods --namespace helmrelease --selector "statefulset.kubernetes.io/pod-name=test1-dgraph-alpha-0,release=test1" --output jsonpath="{.items[0].metadata.name}")
    echo "Access Alpha HTTP/S using http://localhost:8080"
    kubectl --namespace helmrelease port-forward $ALPHA_POD_NAME 8080:8080

   NOTE: Change "http://" to "https://" if TLS was added to the Ingress, Load Balancer, or Dgraph Alpha service.
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ kubectl get all -n test
NAME                       READY   STATUS    RESTARTS   AGE
pod/test1-dgraph-alpha-0   1/1     Running   0          94s
pod/test1-dgraph-alpha-1   1/1     Running   0          94s
pod/test1-dgraph-alpha-2   1/1     Running   0          94s
pod/test1-dgraph-zero-0    1/1     Running   0          94s
pod/test1-dgraph-zero-1    1/1     Running   0          94s
pod/test1-dgraph-zero-2    1/1     Running   0          94s

NAME                                  TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)             AGE
service/test1-dgraph-alpha            ClusterIP   10.96.174.108   <none>        8080/TCP,9080/TCP   95s
service/test1-dgraph-alpha-headless   ClusterIP   None            <none>        7080/TCP            95s
service/test1-dgraph-zero             ClusterIP   10.96.92.83     <none>        5080/TCP,6080/TCP   95s
service/test1-dgraph-zero-headless    ClusterIP   None            <none>        5080/TCP            95s

NAME                                  READY   AGE
statefulset.apps/test1-dgraph-alpha   3/3     95s
statefulset.apps/test1-dgraph-zero    3/3     95s
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ kubectl get all -n helmrelease
No resources found in helmrelease namespace.
@r-t-m ➜ /workspaces/charts (dgraph_namespace) $ helm list --all-namespaces
NAME    NAMESPACE       REVISION        UPDATED                                 STATUS          CHART           APP VERSION
test1   helmrelease     1               2023-11-30 21:09:24.971076025 +0000 UTC deployed        dgraph-0.2.2    v23.0.1

@kevinmingtarja
Copy link
Contributor

Awesome, thanks for sending over the command outputs! The PR looks good, we'll test it again internally and approve it once we're done.

@kevinmingtarja
Copy link
Contributor

kevinmingtarja commented Dec 4, 2023

Hi @r-t-m, I think you missed changing the NOTES.txt. If you see the output you sent, it it still showing up as --namespace helmrelease, as it's still referring to .Release.Namespace. Can you change this as well? Other than that, everything looks good, thanks!

2. Get the Dgraph Alpha HTTP/S endpoint by running these commands.

    export ALPHA_POD_NAME=$(kubectl get pods --namespace helmrelease --selector "statefulset.kubernetes.io/pod-name=test1-dgraph-alpha-0,release=test1" --output jsonpath="{.items[0].metadata.name}")
    echo "Access Alpha HTTP/S using http://localhost:8080"
    kubectl --namespace helmrelease port-forward $ALPHA_POD_NAME 8080:8080

   NOTE: Change "http://" to "https://" if TLS was added to the Ingress, Load Balancer, or Dgraph Alpha service.

I think the same issue is also there in your other PR for ratel.

Signed-off-by: Artem Chubar <57397719+r-t-m@users.noreply.github.com>
Copy link
Contributor

@kevinmingtarja kevinmingtarja left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution @r-t-m !

@kevinmingtarja kevinmingtarja merged commit 1987415 into dgraph-io:master Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants