Skip to content

Conversation

@mikechu-optimizely
Copy link
Contributor

No description provided.

@mikechu-optimizely mikechu-optimizely added the documentation Improvements or additions to documentation label Jun 24, 2022
@mikechu-optimizely
Copy link
Contributor Author

@msohailhussain Am I good to merge this to main?

@mikechu-optimizely
Copy link
Contributor Author

Note: There are TODOs to review for @msohailhussain or @yasirfolio3 to comment

@msohailhussain
Copy link
Contributor

@msohailhussain Am I good to merge this to main?

I will review today. Please wait.

Copy link
Contributor

@msohailhussain msohailhussain left a comment

Choose a reason for hiding this comment

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

mostly looks good please address my comments.

@mikechu-optimizely
Copy link
Contributor Author

@msohailhussain I'm ready for a second code review

I was using this command to test
helm install -f values.yaml --dry-run --debug debugging-only ./

Comment on lines +73 to +89
Run the following in a *nix terminal to access the running
{{- if contains "NodePort" .Values.service.type }}
{{- " NodePort service endpoint" }}
export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "optimizely-agent.fullname" . }})
export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}")
echo http://$NODE_IP:$NODE_PORT
{{- else if contains "LoadBalancer" .Values.service.type }}
{{- " LoadBalancer service endpoint" }}
NOTE: It may take a few minutes for the LoadBalancer IP to be available.
You can watch the status of by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "optimizely-agent.fullname" . }}'
export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "optimizely-agent.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}")
echo http://$SERVICE_IP:{{ .Values.service.port }}
{{- else if contains "ClusterIP" .Values.service.type }}
{{- " ClusterIP service IP address" }}
export CLUSTER_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "optimizely-agent.fullname" . }} -o jsonpath='{.spec.clusterIP}')
echo http://$CLUSTER_IP:8080
{{- end }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@msohailhussain I thought it was good to keep the outputs from the other service types. 😄

My research points to our need to run kubectl commands to access info from the running deployments like IP, Ports, etc. It appears that helm resolves templates earlier before the deployment is necessarily online ie no access to post-install values in the NOTES. ...again, from what I can tell.

I'm certain @yasirfolio3 can take this further than my knowledge/experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have a ticket, and will fix later. For now it's good we may release as it is.

@msohailhussain
Copy link
Contributor

lgtm

Copy link
Contributor

@msohailhussain msohailhussain left a comment

Choose a reason for hiding this comment

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

lgtm

@msohailhussain msohailhussain merged commit 6804972 into main Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants