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

[BUG] generate_helm.sh doesn't update helm docs for all charts #4950

Closed
2 tasks done
lowc1012 opened this issue Feb 25, 2024 · 5 comments
Closed
2 tasks done

[BUG] generate_helm.sh doesn't update helm docs for all charts #4950

lowc1012 opened this issue Feb 25, 2024 · 5 comments
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation helm

Comments

@lowc1012
Copy link
Contributor

lowc1012 commented Feb 25, 2024

Describe the bug

When I updated values.yaml in flyte-binary chart and ran make helm, the README.md didn't be modified

Expected behavior

README.md should be modified if I modified the values.yaml in charts. Need to check if each helm doc can be modified with changed vaules.yaml via generate_helm.sh.

Additional context to reproduce

No response

Screenshots

截圖 2024-02-26 凌晨12 21 37

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@lowc1012 lowc1012 added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Feb 25, 2024
Copy link

dosubot bot commented Feb 25, 2024

🤖

Hey there @lowc1012! Great to see you around here again. 😄 I'll dive into this helm docs conundrum and get back to you with a full answer shortly. Hang tight!

@lowc1012
Copy link
Contributor Author

#take

@dosubot dosubot bot added the documentation Improvements or additions to documentation label Feb 25, 2024
Copy link

dosubot bot commented Feb 25, 2024

🤖

Based on the information you've provided, it seems like you've already run the generate_helm.sh script after making changes to the values.yaml file in the flyte-binary chart, but the README.md was not updated as expected.

The generate_helm.sh script should indeed update the helm docs for all charts, including the flyte-binary chart, when changes are made to the values.yaml file. The script checks if helm-docs is installed, installs it if it's not, and then runs helm-docs on the charts directory. The helm-docs command generates documentation for helm charts based on the values.yaml file and other files in the chart directory. So, any changes made to the values.yaml file should be reflected in the generated documentation when this script is run.

If the README.md for the flyte-binary chart was not updated as expected, it could be due to a few reasons:

  1. There might be an issue with the helm-docs tool itself. You can try updating it to the latest version and see if that resolves the issue.
  2. There might be an issue with the values.yaml file. Make sure it's formatted correctly and doesn't contain any errors.
  3. There might be an issue with the generate_helm.sh script. You can try running the helm-docs command manually on the flyte-binary chart directory and see if that updates the README.md.

If none of these suggestions resolve the issue, it would be helpful if you could provide more information about the changes you made to the values.yaml file and the exact steps you took when running the generate_helm.sh script. This will help in identifying the cause of the issue.

For reference, here is the section of the generate_helm.sh script that generates the helm docs:

echo "Generating helm docs"
if  command -v helm-docs &> /dev/null
then
    rm $(which helm-docs)
fi

GO111MODULE=on go install github.com/norwoodj/helm-docs/cmd/helm-docs@latest

${GOPATH:-~/go}/bin/helm-docs -c ${DIR}/../charts/

And here is the structure of the flyte-binary chart in the Flyte repository:

  • apiVersion: v2
  • name: flyte-binary
  • description: Chart for basic single Flyte executable deployment
  • type: application
  • version: v0.1.10
  • appVersion: 1.16.0
  • dependencies:
    • name: flyteagent
    • condition: flyteagent.enabled
    • alias: flyteagent
    • version: v0.1.10
    • repository: file://../flyteagent

Sources:

Sources

About Dosu

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@lowc1012
Copy link
Contributor Author

lowc1012 commented Feb 25, 2024

Root cause:
In generate_helm.sh, we uses latest norwoodj/helm-docs which requires go1.22
norwoodj/helm-docs@6cdd5ff

But flyte targets go1.21, so that it failed to generate helm-doc
https://github.com/flyteorg/flyte/blob/master/go.mod#L3

@pingsutw pingsutw added helm and removed untriaged This issues has not yet been looked at by the Maintainers labels Feb 29, 2024
@eapolinario eapolinario mentioned this issue Mar 2, 2024
3 tasks
@davidmirror-ops
Copy link
Contributor

Fixed by #4995

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation helm
Projects
None yet
Development

No branches or pull requests

3 participants