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

OCM descriptor as CRD #69

Merged
merged 4 commits into from
Sep 14, 2022
Merged

OCM descriptor as CRD #69

merged 4 commits into from
Sep 14, 2022

Conversation

Skarlso
Copy link
Contributor

@Skarlso Skarlso commented Aug 26, 2022

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #35

Special notes for your reviewer:

Release note:

@gardener-robot
Copy link
Contributor

@Skarlso Thank you for your contribution.

@Skarlso
Copy link
Contributor Author

Skarlso commented Aug 29, 2022

Hmm, hmm. I'm a bit perplexed as to the structure based on the v2, v3 schemas. The question here is whether to embed the Object or not. But if we embed the Object, all the values will have an apiVersion which is not something that will work nicely in Kubernetes.

So, maybe we stay with an approximation instead of a full 1:1 mapping.

@Skarlso Skarlso force-pushed the ocm-crd branch 2 times, most recently from 89c733c to 88ff539 Compare August 29, 2022 07:31
Copy link
Contributor

@mandelsoft mandelsoft left a comment

Choose a reason for hiding this comment

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

Is it possible to put the CRD stuff into a some sub directory?
It is quite prominent, so far. And the structure does not really meet the actual project structure.

Basically, instead of using own types for the various elements the CRD should use the
already existing binding types. Is this possible?

Or alternatively, we need a transfer function, that provides a component descriptor (according to this library) from the K8S resource, and the opposite direction, putting a CD into an K8S resource.

This should be based on the internal structure (package compdesc), not the serialization versions. This would support a smooth usage together with the rest of the lib.

Makefile Outdated Show resolved Hide resolved
api/v2/ocm_descriptor.go Outdated Show resolved Hide resolved
api/v3alpha1/ocm_descriptor.go Outdated Show resolved Hide resolved
@Skarlso
Copy link
Contributor Author

Skarlso commented Aug 29, 2022

Or alternatively, we need a transfer function, that provides a component descriptor (according to this library) from the K8S resource, and the opposite direction, putting a CD into an K8S resource.

Yep, that's how most projects do it. If you have some kind of internal structure you usually do a transformation between that and the k8s representation.

Is it possible to put the CRD stuff into a some sub directory?
It is quite prominent, so far. And the structure does not really meet the actual project structure.

This is the convention for any CRD in any project by the CRD generator codes. We can put it elsewhere of course, but that's not really intuitive, not to mention that some tooling like code-generators might not work appropriately.

I suggest we follow the standards in this case as it's a well-trodden path.

@Skarlso Skarlso marked this pull request as ready for review September 5, 2022 15:06
@Skarlso Skarlso requested a review from a team as a code owner September 5, 2022 15:06
@gardener-robot
Copy link
Contributor

@yitsushi, @In-Ko You have pull request review open invite, please check

@Skarlso Skarlso marked this pull request as draft September 9, 2022 12:12
@Skarlso Skarlso changed the title OCM as CRD [WIP] OCM as CRD Sep 9, 2022
@Skarlso Skarlso force-pushed the ocm-crd branch 2 times, most recently from 8b609bc to 4e7ee00 Compare September 9, 2022 13:51
@Skarlso Skarlso marked this pull request as ready for review September 9, 2022 13:59
@Skarlso
Copy link
Contributor Author

Skarlso commented Sep 9, 2022

This is now ready for review. It got a lot simpler as its reused internal, already existing definitions and types. 🎉

@Skarlso Skarlso requested a review from mandelsoft September 9, 2022 14:01
@Skarlso Skarlso changed the title [WIP] OCM as CRD OCM descriptor as CRD Sep 9, 2022
@gardener-robot
Copy link
Contributor

@In-Ko You have pull request review open invite, please check

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.

Use a K8s custom resource definition to describe the component descriptor
5 participants