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

Rename/Cleanup CLI Flags #872

Open
unmarshall opened this issue Sep 3, 2024 · 0 comments
Open

Rename/Cleanup CLI Flags #872

unmarshall opened this issue Sep 3, 2024 · 0 comments
Labels
area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related area/usability Usability related kind/enhancement Enhancement, improvement, extension

Comments

@unmarshall
Copy link
Contributor

unmarshall commented Sep 3, 2024

How to categorize this issue?

/area open-source
/area quality
/area usability
/kind enhancement

What would you like to be added:

  1. leader-election-resource-lock is marked as deprecated but its value is honoured. Ideally we should ignore the value that is set via this flag. in v0.23.0 this was marked as deprecated and it should be removed in v0.27.0
  2. disable-lease-cache we currently do not know why this flag is required. This is not configured/set in g/g use case.
  3. We should revisit the reasons to have etcd-member-notready-threshold and etcd-member-unknown-threshold. Currently it is not very clear on the need for it and even if these flags are set what are the magical values that one can set is also not very clear.
  4. etcd-events-threshold is meant for compaction controller but its name is too generic. This flag should be renamed as from its name its purpose is not clear at all. Similarly there are other CLI flags which have the same problem: active-deadline-duration, metrics-scrape-wait-duration
  5. Flag reconciler-service-account is very generic but only applies to the etcd controller.
  6. Flag disable-etcd-serviceaccount-automount should be marked deprecated and removed later. Since k8s 1.24, projected service account token is the default. What we therefore need is only for the user to specify a different expirationSeconds.

NOTE: Care should be taken to not leak implementation details via CLI Flags.

Why is this needed:
Many of the etcd-druid CLI args are badly named and they do not clearly indicate what their purpose is. Some of these flags are questionable and should be considered to be marked as deprecated and in future removed. Not having well-defined CLI flags make it not-so-easy to consume druid and only adds cognitive overhead to first understand each of the flags and keep that mapping fresh in your mind as well as their name do not suggest what they really do.

@gardener-robot gardener-robot added area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related area/usability Usability related kind/enhancement Enhancement, improvement, extension labels Sep 3, 2024
@unmarshall unmarshall changed the title Rename CLI Args Rename/Cleanup CLI Args Sep 3, 2024
@unmarshall unmarshall changed the title Rename/Cleanup CLI Args Rename/Cleanup CLI Flags Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related area/usability Usability related kind/enhancement Enhancement, improvement, extension
Projects
None yet
Development

No branches or pull requests

2 participants