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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] allow-collecting-longhorn-usage-metrics setting is missing from chart settings #7050

Closed
yardenshoham opened this issue Nov 6, 2023 · 8 comments 路 Fixed by #7049
Closed
Assignees
Labels
backport/1.5.4 kind/bug require/backport Require backport. Only used when the specific versions to backport have not been definied. require/chart Require updating (chart) manifests in longhorn, longhorn-manager, charts repos require/doc Require updating the longhorn.io documentation require/qa-review-coverage Require QA to review coverage
Milestone

Comments

@yardenshoham
Copy link
Contributor

Describe the bug (馃悰 if you encounter this issue)

The chart is missing the allow-collecting-longhorn-usage-metrics setting. We should add it.

Additional context

cc @innobead @longhorn/qa @c3y1huang

@yardenshoham yardenshoham added kind/bug require/backport Require backport. Only used when the specific versions to backport have not been definied. require/qa-review-coverage Require QA to review coverage labels Nov 6, 2023
@longhorn-io-github-bot
Copy link

Pre Ready-For-Testing Checklist

  • Where is the reproduce steps/test steps documented?
    The reproduce steps/test steps are at:

  • Is there a workaround for the issue? If so, where is it documented?
    The workaround is at:

  • Does the PR include the explanation for the fix or the feature?

  • Does the PR include deployment change (YAML/Chart)? If so, where are the PRs for both YAML file and Chart?
    The PR for the YAML change is at:
    The PR for the chart change is at:

  • Have the backend code been merged (Manager, Engine, Instance Manager, BackupStore etc) (including backport-needed/*)?
    The PR is at

  • Which areas/issues this PR might have potential impacts on?
    Area
    Issues

  • If labeled: require/LEP Has the Longhorn Enhancement Proposal PR submitted?
    The LEP PR is at

  • If labeled: area/ui Has the UI issue filed or ready to be merged (including backport-needed/*)?
    The UI issue/PR is at

  • If labeled: require/doc Has the necessary document PR submitted or merged (including backport-needed/*)?
    The documentation issue/PR is at

  • If labeled: require/automation-e2e Has the end-to-end test plan been merged? Have QAs agreed on the automation test case? If only test case skeleton w/o implementation, have you created an implementation issue (including backport-needed/*)
    The automation skeleton PR is at
    The automation test case PR is at
    The issue of automation test case implementation is at (please create by the template)

  • If labeled: require/automation-engine Has the engine integration test been merged (including backport-needed/*)?
    The engine automation PR is at

  • If labeled: require/manual-test-plan Has the manual test plan been documented?
    The updated manual test plan is at

  • If the fix introduces the code for backward compatibility Has a separate issue been filed with the label release/obsolete-compatibility?
    The compatibility issue is filed at

@innobead innobead added this to the v1.6.0 milestone Nov 6, 2023
@innobead innobead reopened this Dec 4, 2023
@innobead
Copy link
Member

innobead commented Dec 4, 2023

@ChanYiLin this is contributed by @yardenshoham, because he does not belong to Longhorn org, please help drive this issue to walk through the issue process. Also, see if there is any doc we need to update and update accordingly.

@ChanYiLin
Copy link
Contributor

ChanYiLin commented Dec 4, 2023

Hi @yardenshoham
Thanks for your contribution
After the setting is added to the helm chart, we can then further update the content in our website.
Could you help to create a PR to this Repo: https://github.com/longhorn/website

  1. Paste the helm values table you generated in README to this page
  2. Add the description of the setting to this page

And that's it, thanks!

@yardenshoham
Copy link
Contributor Author

I have a branch with 1 done, I'm not sure where exactly 2 should go because it's a default setting

@ChanYiLin
Copy link
Contributor

yes now we have to update the content in our website so users will see the new setting when they refer to our website
you can go to this repo: https://github.com/longhorn/website
and create a PR to update the helm values table you generated to this place place:https://github.com/longhorn/website/blob/master/content/docs/1.6.0/references/helm-values.md

@yardenshoham
Copy link
Contributor Author

Done longhorn/website#806

@ChanYiLin
Copy link
Contributor

ChanYiLin commented Dec 5, 2023

Hi @yardenshoham
Now our QA team will test the helm chart and close the issue if it all works fine.
Thanks for your contribution again!

@roger-ryao roger-ryao added require/doc Require updating the longhorn.io documentation require/chart Require updating (chart) manifests in longhorn, longhorn-manager, charts repos labels Dec 5, 2023
@roger-ryao
Copy link

Verified on master-head 20231205

Result Passed

  • require/chart : Installed Longhorn using Helm chart and modified allowCollectingLonghornUsageMetrics to false, and it took effect.
  • require/doc : Checked documentation in Update helm-chart values聽website#806

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.5.4 kind/bug require/backport Require backport. Only used when the specific versions to backport have not been definied. require/chart Require updating (chart) manifests in longhorn, longhorn-manager, charts repos require/doc Require updating the longhorn.io documentation require/qa-review-coverage Require QA to review coverage
Projects
Status: Resolved/Scheduled
Development

Successfully merging a pull request may close this issue.

5 participants