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

Use go-bindata to embed YAML files with CRDs. #1211

Merged
merged 7 commits into from
Dec 20, 2019
Merged

Conversation

porridge
Copy link
Member

@porridge porridge commented Dec 19, 2019

What this PR does / why we need it:

This is the first big step towards #862.

It temporarily makes YAML files that reside in config/crds the authoritative source for the KUDO CRDs. At the same time it drops the duplicate definition in the kudoinit package. Note that there is no user visible change (none of the golden files are updated).

The next step to follow soon will be to make these YAML files auto-generated as well, by generating them from the structs in the apis sub-package, completing the journey to a single human-edited source of truth.

Since these files are are the authoritative source now, we remove the option of auto-updating them in the integration test. However we keep the test around as a way of making sure the file generated by go-bindata in make generate is up-to-date.

It was never used (verified by eyeballing `git log -p` all the way
down), we don't seem to know what it's for, and it has no counterpart in
the structs.
This is in preparation for moving to `controller-gen`, which is somewhat
opinionated in that it does not allow us to pick the exact file names.

I guess we do not really care what these are called anyway - the only
place where these names show up is this integration test.
Also add go-bindata to the `generate` target, and a README.
They are the authoritative *source* now.

Keep the test around as a way of making sure the file generated by
go-bindata is up-to-date.
Copy link
Contributor

@zen-dog zen-dog left a comment

Choose a reason for hiding this comment

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

Nice work!

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

Loving this 👍

@porridge porridge changed the base branch from rename-crds to master December 20, 2019 09:32
@porridge porridge merged commit 8b55215 into master Dec 20, 2019
@porridge porridge deleted the use-go-bindata branch December 20, 2019 10:16
@kensipe
Copy link
Member

kensipe commented Dec 30, 2019

@porridge this seems backwards... but I'm anxious to see the end result. The CRD files in the folders were suppose to be removed. There were issues blocking us when init was added in... but I think those do not exist any more. This generated file keeps unexpectedly showing changes in the generated go file. :(

ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
* Remove the unused crdVersion from CRD.

It was never used (verified by eyeballing `git log -p` all the way
down), we don't seem to know what it's for, and it has no counterpart in
the structs.

* Rename the CRD files.

This is in preparation for moving to `controller-gen`, which is somewhat
opinionated in that it does not allow us to pick the exact file names.

I guess we do not really care what these are called anyway - the only
place where these names show up is this integration test.

* goimports

* Use go-bindata to embed YAML files with CRDs.

Also add go-bindata to the `generate` target, and a README.

* Remove option to write manifests from test.

They are the authoritative *source* now.

Keep the test around as a way of making sure the file generated by
go-bindata is up-to-date.

* Update README.

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
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

4 participants