-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
ansible/helm : align flags and deprecate the old ones #4654
ansible/helm : align flags and deprecate the old ones #4654
Conversation
# endpoint w/o any authn/z, please comment the following line. | ||
# Protect the /metrics endpoint by putting it behind auth. | ||
# If you want your controller-manager to expose the /metrics | ||
# endpoint w/o any authn/z, please comment the following line. |
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.
these changes are because of the alignment to keep the file such as upstream.
&prometheus.Kustomization{}, | ||
&prometheus.ServiceMonitor{}, | ||
&kdefault.AuthProxyPatch{}, | ||
&kdefault.ManagerAuthProxyPatch{}, |
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.
these changes are because of the alignment to keep the file such as upstream.
just replace the names
type Kustomization struct { | ||
file.TemplateMixin | ||
} | ||
|
||
// SetTemplateDefaults implements input.Template | ||
// SetTemplateDefaults implements file.Template | ||
func (f *Kustomization) SetTemplateDefaults() error { |
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.
these changes are because of the alignment to keep the file such as upstream.
// Manager scaffolds yaml config for the manager. | ||
type Manager struct { | ||
// Config scaffolds a file that defines the namespace and the manager deployment | ||
type Config struct { |
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.
these changes are because of the alignment to keep the file such as upstream.
Hi @estroz, Really thank you. All suggestion (less this nit #4654 (comment)) are addressed, PLease, feel free to check. |
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.
Changelog nits, otherwise LGTM.
# release notes and/or the migration guide | ||
entries: | ||
- description: > | ||
(ansible/v1)(helm/v1) In order to align the Ansible/Helm based-projects with the Kubernetes nomenclatures the flags `--enable-leader-election` and `--metrics-addr` were deprecated and replaced by `--leader-elect` and `--metrics-bind-address` respectively. |
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.
(ansible/v1)(helm/v1) In order to align the Ansible/Helm based-projects with the Kubernetes nomenclatures the flags `--enable-leader-election` and `--metrics-addr` were deprecated and replaced by `--leader-elect` and `--metrics-bind-address` respectively. | |
(ansible/v1, helm/v1) The flags `--enable-leader-election` and `--metrics-addr` were deprecated in favor of `--leader-elect` and `--metrics-bind-address`, respectively, to follow upstream conventions. |
# Migration can be defined to automatically add a section to | ||
# the migration guide. This is required for breaking changes. | ||
migration: | ||
header: (ansible/v1)(helm/v1) Replace the deprecated flags |
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.
header: (ansible/v1)(helm/v1) Replace the deprecated flags | |
header: (ansible/v1, helm/v1) Replace deprecated leader election and metrics address flags |
migration: | ||
header: (ansible/v1)(helm/v1) Replace the deprecated flags | ||
body: > | ||
Replace the flags `--enable-leader-election` and `--metrics-addr` which were deprecated with `--leader-elect` and `--metrics-bind-address` respectively. |
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.
Replace the flags `--enable-leader-election` and `--metrics-addr` which were deprecated with `--leader-elect` and `--metrics-bind-address` respectively. | |
Replace deprecated flags `--enable-leader-election` and `--metrics-addr` with `--leader-elect` and `--metrics-bind-address`, respectively. |
body: > | ||
Replace the flags `--enable-leader-election` and `--metrics-addr` which were deprecated with `--leader-elect` and `--metrics-bind-address` respectively. | ||
- description: > | ||
(ansible/v1)(helm/v1) Ensure that the `--health-probe-bind-address` port will be informed to the proxy. |
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.
(ansible/v1)(helm/v1) Ensure that the `--health-probe-bind-address` port will be informed to the proxy. | |
(ansible/v1, helm/v1) Explicitly set `--health-probe-bind-address` in the manager's auth proxy patch. |
# - deprecation | ||
# - removal | ||
# - bugfix | ||
kind: "bugfix" |
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.
This is more of a non-breaking change, since nothing is currently broken.
kind: "bugfix" | |
kind: "change" |
# Migration can be defined to automatically add a section to | ||
# the migration guide. This is required for breaking changes. | ||
migration: | ||
header: (ansible/v1)(helm/v1) Ensure that the `--health-probe-bind-address` port will be informed to the proxy. |
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.
header: (ansible/v1)(helm/v1) Ensure that the `--health-probe-bind-address` port will be informed to the proxy. | |
header: (ansible/v1, helm/v1) Explicitly set `--health-probe-bind-address` in the manager's auth proxy patch. |
Signed-off-by: Camila Macedo <cmacedo@redhat.com>
@estroz All done tks. |
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.
/lgtm
This is related to upgrade operator-sdk to v1.6.0: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.6.0/ operator-framework/operator-sdk#4654 Signed-off-by: Wayne Sun <gsun@redhat.com>
Description of the change:
Motivation for the change:
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments
(seechangelog/fragments/00-template.yaml
)website/content/en/docs