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

DOCS-1021: Attempting to add autoyaml docs for Helm charts #1827

Merged
merged 3 commits into from
Nov 10, 2023
Merged

Conversation

ravindk89
Copy link
Contributor

Part of supporting minio/docs#1021

Summary

We've had an increase of requests for guidance in deploying via Helm.

This attempts to formalize the documentation structure for the Values.yaml in a format that can be automatically parsed by Sphinx into some form of web documentation.

It also attempts to synchronize some of the functionality between the Chart and the CRD.

Before merging either of these, we need to validate that the chart deployment itself is unaffected by the comments.

I've left comments for those areas which were unclear in their functionality.

@ravindk89 ravindk89 self-assigned this Oct 20, 2023
helm/operator/values.yaml Show resolved Hide resolved
helm/operator/values.yaml Show resolved Hide resolved
helm/tenant/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Show resolved Hide resolved
helm/tenant/values.yaml Show resolved Hide resolved
@ravindk89
Copy link
Contributor Author

@pjuarezd @cniackz let me know what you think of the above.

@ravindk89 ravindk89 requested a review from cniackz November 1, 2023 18:42
@cniackz
Copy link
Contributor

cniackz commented Nov 1, 2023

Let's have test passing to approve it:

Change the version accordingly for image to be found within the cluster
Error: bad file '/home/runner/work/operator/operator/testing/../helm/operator/values.yaml': yaml: line 163: did not find expected key
Error: bad file '/home/runner/work/operator/operator/testing/../helm/operator/values.yaml': yaml: line 163: did not find expected key
Error: bad file '/home/runner/work/operator/operator/testing/../helm/operator/values.yaml': yaml: line 163: did not find expected key
Error: bad file '/home/runner/work/operator/operator/testing/../helm/operator/values.yaml': yaml: line 163: did not find expected key

helm/tenant/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Show resolved Hide resolved
helm/tenant/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Show resolved Hide resolved
@ravindk89 ravindk89 requested a review from pjuarezd November 7, 2023 19:52
@ravindk89
Copy link
Contributor Author

OK @pjuarezd if this looks good we can merge it here, and I can finish the import work on the actual docs PR. Assuming all changes work.

pjuarezd
pjuarezd previously approved these changes Nov 7, 2023
@pjuarezd
Copy link
Member

pjuarezd commented Nov 7, 2023

OK @pjuarezd if this looks good we can merge it here, and I can finish the import work on the actual docs PR. Assuming all changes work.

Looks good to me, we need another approver to merge tho

@ravindk89
Copy link
Contributor Author

#1852 should resolve the testing errors.

@pjuarezd
Copy link
Member

pjuarezd commented Nov 8, 2023

#1852 should resolve the testing errors.

that PR is merged already, could you rebase please @ravindk89?

pjuarezd
pjuarezd previously approved these changes Nov 9, 2023
@ravindk89 ravindk89 requested review from feorlen and djwfyi November 10, 2023 15:54
helm/operator/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Outdated Show resolved Hide resolved
helm/tenant/values.yaml Outdated Show resolved Hide resolved
feorlen
feorlen previously approved these changes Nov 10, 2023
Copy link
Contributor

@feorlen feorlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple typos and a reword for your consideration. 🚀

@ravindk89 ravindk89 dismissed stale reviews from feorlen and pjuarezd via 79992dd November 10, 2023 16:42
feorlen
feorlen previously approved these changes Nov 10, 2023
helm/tenant/values.yaml Outdated Show resolved Hide resolved
@pjuarezd pjuarezd merged commit 2515162 into master Nov 10, 2023
25 checks passed
@pjuarezd pjuarezd deleted the DOCS-1021 branch November 10, 2023 18:17
# export MINIO_ROOT_PASSWORD=ROOTUSERPASSWORD
#
existingSecret:
name: myminio-env-configuration

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revise this change. I couldn't find any usage of .Values.existingSecret in tenant Helm templates. There's usage of .Values.secrets.existingSecret instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants