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(chart): templatize/update README and NOTES #155

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

vdice
Copy link
Contributor

@vdice vdice commented Mar 8, 2024

  • Templatize the chart README.md for injecting app and chart version at publish time
  • Update NOTES.txt

Comment on lines 46 to 52
$ kubectl apply -f https://github.com/spinkube/spin-operator/releases/download/{{ APP_VERSION }}/spin-operator.shim-executor.yaml
```

- A RuntimeClass resource for the `wasmtime-spin-v2` container runtime is installed. This is the runtime that Spin applications use.

<!-- TODO: templatize with release version corresponding to chart's appVersion -->

```console
$ kubectl apply -f https://github.com/spinkube/spin-operator/releases/download/v0.1.0-rc.1/spin-operator.runtime-class.yaml
$ kubectl apply -f https://github.com/spinkube/spin-operator/releases/download/{{ APP_VERSION }}/spin-operator.runtime-class.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

these might be better coming from config/samples (via http) - coming from a release artifact implies they are "valid production resources" to me - which they are explicitly not 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

yea I think it might be nice to keep paths to files on main and then for releasing, maybe we rely on sed without the templating?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made some updates in d64faa3. A bit awkward what with converting multiple CRD GH urls to the one combined yaml url attached to a release... and now I realize for all of the pre-release charts we publish from main, we'll be pointing to non-existent GH releases in this README.

Taking a step back, I wonder if we just replace most of the instructions in this README with a link to https://www.spinkube.dev/docs/spin-operator/installation/installing-with-helm/ and do away with the maintenance burden and duplication of instructions? WDYT?

Makefile Outdated
sed -r -i.bak -e 's%^version: .*%version: $(CHART_VERSION)%g' $(STAGING_DIR)/$(CHART_NAME)-$(CHART_VERSION)/Chart.yaml
sed -r -i.bak -e 's%^appVersion: .*%appVersion: "$(APP_VERSION)"%g' $(STAGING_DIR)/$(CHART_NAME)-$(CHART_VERSION)/Chart.yaml

@# Update README.md with CHART_VERSION and APP_VERSION
sed -r -i.bak -e 's%{{ CHART_VERSION }}%$(CHART_VERSION)%g' $(STAGING_DIR)/$(CHART_NAME)-$(CHART_VERSION)/README.md
Copy link
Contributor

Choose a reason for hiding this comment

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

does the .bak file need to be cleaned up? We should also probably add .bak to.gitignore if it ends up in the current directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't seen these linger when running the targets locally, so I think we're good. The main agent using these will be ephemeral GH runners on releases.

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason to create backup files at all when originals are in Git? (I personally don't love them in automation - esp given that we repeatedly overwrite the same backup anyway)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, at one point I thought some versions of sed required a backup file name. But that could be outdated knowledge. I'll test updating it to just -i '' and see if GH runners are happy (it works fine on my Mac).

Copy link
Contributor Author

@vdice vdice Mar 11, 2024

Choose a reason for hiding this comment

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

Darn, so the GH runner doesn't like this approach

sed -r -i '' -e 's%^version: .*%version: 0.0.2-vdice-test%g' _dist/spin-operator-0.0.2-vdice-test/Chart.yaml
sed: can't read : No such file or directory

Copy link
Contributor

Choose a reason for hiding this comment

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

sed -i and sed -i'' are the GNU sed versions - i guess I can live with backups if we want BSD sed to work 😅

Comment on lines 46 to 52
$ kubectl apply -f https://github.com/spinkube/spin-operator/releases/download/{{ APP_VERSION }}/spin-operator.shim-executor.yaml
```

- A RuntimeClass resource for the `wasmtime-spin-v2` container runtime is installed. This is the runtime that Spin applications use.

<!-- TODO: templatize with release version corresponding to chart's appVersion -->

```console
$ kubectl apply -f https://github.com/spinkube/spin-operator/releases/download/v0.1.0-rc.1/spin-operator.runtime-class.yaml
$ kubectl apply -f https://github.com/spinkube/spin-operator/releases/download/{{ APP_VERSION }}/spin-operator.runtime-class.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

yea I think it might be nice to keep paths to files on main and then for releasing, maybe we rely on sed without the templating?

@vdice vdice force-pushed the docs/chart-notes-readme-updates branch 6 times, most recently from 8348b08 to 81dff8e Compare March 11, 2024 21:22
@vdice
Copy link
Contributor Author

vdice commented Mar 11, 2024

I've turned this PR into a hot mess, sorry 😂

Latest changes include:

  • use raw GH urls everywhere. This is simplest for now, instead of trying to swap in GH release urls, especially when in one case we have multiple files but in the other we have one consolidated file (CRDs).
    • for these urls, point to main for source control version and for pre-release chart artifacts (eg merges to main)
    • else, point to semver tag when APP_VERSION matches v*

CI runs looking good on fork: https://github.com/vdice/spin-operator/actions/runs/8242543035/job/22541713719 and https://github.com/vdice/spin-operator/actions/runs/8242537875

For a potential follow-up, as mentioned in #155 (comment), we might consider linking to installation docs at spinkube.dev for parts of the README.md and NOTES.txt instead, where much of the information is duplicated.

Or, we can go with this and revisit things in the near future as we get time and/or brighter thinking (shouldn't be too hard!) Also totally fine with closing this one and starting anew with a diff approach 🧹

@vdice vdice force-pushed the docs/chart-notes-readme-updates branch 2 times, most recently from 1647081 to b1d8255 Compare March 12, 2024 02:07
Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
@vdice vdice force-pushed the docs/chart-notes-readme-updates branch from b1d8255 to 6fe551f Compare March 12, 2024 02:13
@endocrimes endocrimes merged commit 4da3ffa into spinkube:main Mar 12, 2024
11 checks passed
@vdice vdice deleted the docs/chart-notes-readme-updates branch March 13, 2024 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants