-
Notifications
You must be signed in to change notification settings - Fork 16.7k
Conversation
/assign @scottrigby |
cc: @unguiculus |
/assign @viglesiasce |
stable/envoy/Chart.yaml
Outdated
sources: | ||
- https://github.com/envoyproxy/envoy | ||
maintainers: | ||
- name: Joshua M. Dotson |
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.
Use Github username
{{- end }} | ||
{{- end }} | ||
annotations: | ||
checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }} |
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 $
is unnecessary here.
stable/envoy/values.yaml
Outdated
replicaCount: 2 | ||
|
||
podDisruptionBudget: | ||
maxUnavailable: 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.
If you want to configure a default, do it like this:
podDisruptionBudget: |
maxUnavailable: 1
Otherwise, a user would not be able to use minAvailable
if they wanted.
stable/envoy/values.yaml
Outdated
# service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled: "true" | ||
# service.beta.kubernetes.io/aws-load-balancer-internal: "true" | ||
ports: | ||
n0: {port: 10000, targetPort: n0, protocol: TCP} |
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.
It's common practice here to use this notation:
n0:
port: 10000
targetPort: n0
protocol: TCP
I'd suggest you change it everywhere.
/assign |
Thanks for your review @unguiculus . Will fix up and push asap. |
@unguiculus Rebased and pushed a commit with things per your review. Thanks! |
/ok-to-test |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: josdotso, unguiculus 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 |
* [stable/envoy] chart version 1.0.0 * [stable/envoy] Remove trailing spaces from values.yaml * [stable/envoy] Apply fixups per unguiculus' review of 1.0.0
…configmap * artifactory-ha-rbac-support: (60 commits) updated readme RBAC support upgrade alertmanager, prometheus, configmap-reload and node-exporter (helm#6276) Add support for AWS Secrets Manager (helm#6245) Dokuwiki - Several improvements (helm#6326) Mediawiki fix probes & improve notes (helm#6320) Odoo - Several improvements (helm#6316) Joomla! - Several improvements (helm#6322) update signalfx-agent helm chart to v0.1.2 to deploy agent v3.2.2 (helm#6202) cert-manager: fast-forward to upstream f804cb56 (helm#6308) [stable/concourse] correct the parameter name (helm#6283) [stable/envoy] chart version 1.0.0 (helm#5955) nginx-ingress/values.yaml: Updated tag to 0.15.0. (helm#5947) [stable/keycloak] Documentation fixes (helm#6289) [sumologic-fluentd] Update to latest image and expose new configuration to control stat watcher on fluentD in_tail plugins. (helm#6304) Adds tolerations labels for Jaeger pods (helm#6255) Upgrade image version for kube-slack (helm#6305) Add katafygio chart (helm#5543) [stable/k8s-spot-rescheduler] Add support for node selector and tolerations (helm#5789) schema-registry - Templating for liveness/readiness probes (helm#6301) ... # Conflicts: # stable/artifactory-ha/Chart.yaml # stable/artifactory-ha/README.md # stable/artifactory-ha/values.yaml
* [stable/envoy] chart version 1.0.0 * [stable/envoy] Remove trailing spaces from values.yaml * [stable/envoy] Apply fixups per unguiculus' review of 1.0.0
* [stable/envoy] chart version 1.0.0 * [stable/envoy] Remove trailing spaces from values.yaml * [stable/envoy] Apply fixups per unguiculus' review of 1.0.0 Signed-off-by: voron <av@arilot.com>
What this PR does / why we need it: https://www.cncf.io/blog/2017/09/13/cncf-hosts-envoy/