-
Notifications
You must be signed in to change notification settings - Fork 1.5k
adding additional examples on how to use annotations and healthchecks #861
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
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Hi @sc-chad. Thanks for your PR. I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sc-chad If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed the CLA. |
This is awesome! I'll take a deeper look later today 👍 |
/ok-to-test |
metadata: | ||
name: config-map | ||
labels: | ||
application: web |
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.
better to be app.kubernetes.io/name: rails-api
, list of recommended labels: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
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.
I've had issues with lookups on name versus app.kubernetes.io/name. It's probably my own understanding and how to use them. I do need to fix the labels on this config. Will add a PR.
name: config-map | ||
labels: | ||
application: web | ||
namespace: some-name |
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.
better to be an explicit name like rails-api-example
, wonder whether we should use kustomize to layout these examples
alb.ingress.kubernetes.io/certificate-arn: acm:certificate:arn | ||
alb.ingress.kubernetes.io/scheme: internet-facing | ||
alb.ingress.kubernetes.io/subnets: 123456789,95481321,321457987 | ||
alb.ingress.kubernetes.io/security-groups: sg-23157498 |
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.
alb.ingress.kubernetes.io/security-groups
, alb.ingress.kubernetes.io/security-subnets
alb.ingress.kubernetes.io/ssl-policy
should be removed. These three all have sensible default&auto-discover functionality.
Our goal is to make number annotations be as less as possible.
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.
I added them because it wasn't intuitive that they were auto discoverable when we first walking through the examples. I can add some commenting around these to show they are optional and should be auto discovered if you've taken the appropriate steps.
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.
What if they were included but commented out?
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.
Done.
@sc-chad But the knowledges are valuable, like how to do external-DNS, HPA, etc. I think it's worth to break this example into smaller pieces, with each one focus on specific tasks. |
Added in some commenting and removed dependencies. |
@M00nF1sh Anything else you want me to cover here? |
…ontroller into rails-api-examples
kind: Deployment | ||
name: web-external | ||
minReplicas: 1 | ||
maxReplicas: 1 |
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.
Why HPA with min 1 max 1? it will recreate the pod if it reaches 50% cpu?
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.
To reduce the resources required to demo, absolutely could be 1min 2 max
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle rotten |
/remove-lifecycle stale |
/retest |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
* Feature: Deferred TGB queue for no-op reconciles * account for pods that go unready and revive themself * improve logging * preempt pod readiness gate changes by clearing check point * cut v2.9.1 release --------- Co-authored-by: Zachary Nixon <[email protected]> Co-authored-by: Kubernetes Prow Robot <[email protected]>
I added two real examples on how we use the ALB Ingress controller both on internal application load balancers and external application load balancers.
I was unsure on where you would like to link to these examples, but please link to or use however you would like.