-
Notifications
You must be signed in to change notification settings - Fork 61
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
solution #59
base: main
Are you sure you want to change the base?
solution #59
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on meeting the core requirements of the task! 🎉 Your configuration files are well-structured and mostly correct. There are a few minor issues to address, such as correcting the typo in the HPA configuration and verifying probe paths. Additionally, consider the security implications of certain settings and ensure the mateapp
namespace exists. These improvements will enhance the robustness and security of your setup. Keep up the good work! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
apiVersion: kubelet.config.k8s.io/v1beta1 | ||
kind: KubeletConfiguration | ||
evictionHard: | ||
nodefs.available: "0%" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting nodefs.available
to "0%" might lead to issues as it effectively disables eviction based on available node filesystem space. Consider setting a more reasonable threshold to prevent potential node failures due to disk space exhaustion.
patch: | | ||
- op: add | ||
path: /apiServer/certSANs/- | ||
value: my-hostname |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The certSANs
value my-hostname
should be replaced with an actual hostname or IP address that the API server will be accessed from. This is important for ensuring proper certificate validation.
kind: InitConfiguration | ||
nodeRegistration: | ||
kubeletExtraArgs: | ||
node-labels: "ingress-ready=true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The node label ingress-ready=true
is applied to the control-plane node. Ensure that this is intentional, as typically, ingress controllers are deployed on worker nodes.
kind: Deployment | ||
metadata: | ||
name: todoapp | ||
namespace: mateapp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the mateapp
namespace exists in your cluster before applying this deployment. If it doesn't exist, you will need to create it first.
- containerPort: 8080 | ||
livenessProbe: | ||
httpGet: | ||
path: api/health |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The liveness probe path api/health
should be verified to ensure it correctly reflects the health check endpoint of your application. If the path is incorrect, the application might be restarted unnecessarily.
periodSeconds: 5 | ||
readinessProbe: | ||
httpGet: | ||
path: api/ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, verify the readiness probe path api/ready
to ensure it matches the readiness check endpoint of your application. Incorrect paths can lead to the application being marked as not ready.
name: memory | ||
target: | ||
type: Utilization | ||
avrageUtilization: 70 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the field name avrageUtilization
. It should be averageUtilization
. This typo will cause the HPA configuration to be invalid and should be corrected.
- --secure-port=10250 | ||
- --kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname | ||
- --kubelet-use-node-status-port | ||
- --kubelet-insecure-tls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The argument --kubelet-insecure-tls
is used to skip TLS verification when connecting to the kubelet. This can be a security risk as it allows for potential man-in-the-middle attacks. Consider using a secure connection with proper certificates if possible.
spec: | ||
group: metrics.k8s.io | ||
groupPriorityMinimum: 100 | ||
insecureSkipTLSVerify: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The insecureSkipTLSVerify: true
setting is a security risk as it disables TLS verification for the APIService. It's recommended to use valid certificates and set this to false
to ensure secure communication.
No description provided.