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

solved #35

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

solved #35

wants to merge 2 commits into from

Conversation

NikitaLafinskiy
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

Something went wrong while processing the review.
[CODE: 2]

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on setting up the Helm charts and ensuring that most configurations are in place! 🎉 While there are a few areas where additional details or error handling could enhance the robustness and clarity of your setup, these are minor and do not impact the core functionality. Keep up the good work, and consider the suggestions for future improvements! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Comment on lines +7 to +8
dependencies:
- name: mysql

Choose a reason for hiding this comment

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

You need to specify the repository URL for the MySQL dependency. Add a repository field under the mysql dependency to indicate where Helm can find this chart.

kind: Secret
metadata:
name: {{ .Chart.Name }}-secret
namespace: {{ .Values.common.namespace }}

Choose a reason for hiding this comment

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

Ensure that .Values.common.namespace is defined in your values.yaml file. If it's not set, the Secret might not be created in the intended namespace.

kind: StatefulSet
metadata:
name: {{ .Chart.Name }}-stateful-set
namespace: {{ .Values.common.namespace }}

Choose a reason for hiding this comment

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

Ensure that .Values.common.namespace is defined in your values.yaml file. If it's not set, the StatefulSet might not be created in the intended namespace.

spec:
selector:
matchLabels:
app: {{ .Values.mysql.labelName }}

Choose a reason for hiding this comment

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

Ensure that .Values.mysql.labelName is defined in your values.yaml file. This is necessary for proper label matching.

selector:
matchLabels:
app: {{ .Values.mysql.labelName }}
replicas: {{ .Values.statefulSet.replicas }}

Choose a reason for hiding this comment

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

Ensure that .Values.statefulSet.replicas is defined in your values.yaml file to specify the number of replicas for the StatefulSet.

Comment on lines +8 to +10
{{- range $k, $v := .Values.secret }}
{{ $k }}: {{ $v | b64enc | quote }}
{{- end }}

Choose a reason for hiding this comment

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

Ensure that .Values.secret is defined in your values.yaml file. This is necessary for populating the Secret with the correct data.

kind: Ingress
metadata:
name: {{ .Chart.Name }}-ingress
namespace: {{ .Values.common.namespace }}

Choose a reason for hiding this comment

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

Ensure that .Values.common.namespace is defined in your values.yaml file. If it's not set, the Ingress might not be created in the intended namespace.

@@ -0,0 +1 @@
In order to validate the cluster has been launched successfully make a request to http://localhost/

Choose a reason for hiding this comment

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

Consider adding more detailed instructions on what the expected response should be when making a request to http://localhost/. Additionally, mention any prerequisites or setup steps required before making this request, such as port forwarding or ensuring the application is running.

kubectl apply -f .infrastructure/app/clusterIp.yml
kubectl apply -f .infrastructure/app/nodeport.yml
kubectl apply -f .infrastructure/app/hpa.yml
kubectl apply -f .infrastructure/app/deployment.yml

# Install Ingress Controller
kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/main/deploy/static/provider/kind/deploy.yaml

Choose a reason for hiding this comment

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

Consider adding error handling after this command to verify that the Ingress Controller was applied successfully. You can use kubectl wait or check the status of the deployment.

# kubectl apply -f .infrastructure/ingress/ingress.yml

# Install the Todoapp and MySQL Helm charts
helm install todoapp-release .

Choose a reason for hiding this comment

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

Consider adding error handling to ensure that the Helm chart installation is successful. You can check the exit status of the helm install command and provide feedback if it fails.

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

Successfully merging this pull request may close these issues.

2 participants