Repository navigation
feat: add --logs-collector-enabled flag to kubernetes create Fixes #485 - #596
yuvarajrece wants to merge 52 commits into
Conversation
Added a flag to enable or disable logs collection for the Civo platform dashboard.
Added logs collector feature to Kubernetes creation command.
|
@alessandroargentieri @alejandrojnm Kindly review the pr and approve it |
Added a 'quiet' flag to suppress prompts and output.
Add error handling for missing name and API key in quiet mode.
Fix missing newline at end of file
Replace spinner initialization with utility.NewSpinner and add quiet mode check for output.
Removed logs collector functionality and updated spinner usage.
Add Quiet option to suppress prompts and output.
Added common.Quiet checks to suppress messages in quiet mode.
Added handling for --quiet mode in AskForConfirm function.
Implement NewSpinner function for progress indication.
|
@alessandroargentieri @alejandrojnm @fulviodenza Review this code |
fulviodenza
left a comment
There was a problem hiding this comment.
Thanks for working on #485, but this PR can't be merged in its current state and I think it is best to restart it from a clean branch.
- It doesn't compile.
cmd/kubernetes/kubernetes.gobinds--logs-collector-enabledtologsCollectorEnabled, but a later commit ("Refactor kubernetes_create.go to remove logs collector") deleted both that variable and theResolveLogsCollectorEnabledcall, so the build fails withundefined: logsCollectorEnabledand Lint/Security/Test are red. The logs collector feature itself is effectively no longer in the PR. - It depends on an unmerged civogo change.
configKubernetes.LogsCollectorEnableddoesn't exist in the civogo version pinned ingo.mod(v0.7.2), so even before that refactor this could not build. The civogo side (civo/civogo#239) needs to be merged and released first, thengo.modbumped here. The PR body still sayscivo/civogo#<PR_NUMBER_HERE>. - Unrelated changes. Roughly 40 of the 46 files implement a global
--quiet/-qflag (#497), not #485. That changes behaviour well beyond this feature (for example "Operation aborted." moves from stdout to stderr with anError:prefix, and--quietwithout--yesaborts with exit 0 and hides the reason). It deserves its own PR and review. - Branch. The PR is opened from your fork's
master, which is why later work got mixed in. Please use a dedicated feature branch per change.
Suggested path: one PR for the logs collector flag (once civogo has the field), with a test that kubernetes create actually wires the flag into the request, and a separate PR for --quiet if you'd like to pursue #497. A couple of small notes for the redo: the user-facing message should name the real flag (--logs-collector-enabled=true) rather than LogsCollectorEnable, and IsStandardSmallOrXSmallKubeSize should only match kubernetes node sizes.
I'm going to request changes here; feel free to close this one and open fresh PRs.
Summary
Fixes #485
Adds a
--logs-collector-enabledflag tocivo kubernetes create,allowing users to control whether the logs collector is installed on a
new cluster.
regardless of node size.
standard small/xsmall nodes (
g4s.kube.small,g4s.kube.xsmall) tosave node resources, and an informational message is printed:
"Logs collector not installed by default to save resource on small and
xsmall standard nodes. If you wish to enable logs collector create the
cluster with LogsCollectorEnable set to true"
API's own default applies.
Depends on civo/civogo#<PR_NUMBER_HERE> for the underlying
LogsCollectorEnabledfield onKubernetesClusterConfig— needs to bemerged and released before this PR can build against a real civogo
version bump (currently uses a local
replaceonly for testing, removedbefore submission).
Testing
Added unit tests in
utility/kubernetes_test.gocovering node-sizedetection and the enable/disable resolution logic (explicit flag,
auto-disable on standard small/xsmall, default-deferred on other sizes,
and confirming non-standard tiers like performance/RAM/CPU-optimized are
not affected).