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

Continued evolution of the package variant design #3910

Merged
merged 17 commits into from
Apr 25, 2023

Conversation

johnbelamaric
Copy link
Contributor

This PR will add additional details, including more on PackageVariantSet.

@johnbelamaric johnbelamaric requested a review from a team as a code owner April 7, 2023 20:44
@johnbelamaric johnbelamaric marked this pull request as draft April 7, 2023 20:45
docs/design-docs/08-package-variant.md Outdated Show resolved Hide resolved

![Figure 5: List of Targets](packagevariantset-target-package.png)
Rules for generating a PackageVariant are associated with a list of targets
using a template. That template can have explicit values for various
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about the use of the word template here. I was at first thinking it would be similar to the PodSpec template in a Deployment resource, but this is somewhat different. At first this seems more like an overlay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's see what you think once I publish this. It's a lot like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, take a look at the complete API as published, see what you think. I think template is appropriate.

docs/design-docs/08-package-variant.md Outdated Show resolved Hide resolved
docs/design-docs/08-package-variant.md Show resolved Hide resolved
@johnbelamaric johnbelamaric marked this pull request as ready for review April 11, 2023 21:45
@johnbelamaric
Copy link
Contributor Author

Marking this ready for review. Still to do before we move from "Work In Progress" to "Approved":

  • fill in the PV status section
  • fill in the PVS status section
  • fill in the use cases

@johnbelamaric
Copy link
Contributor Author

I ended up pulling separate use case list out, I discuss a few examples throughout the rest of the design. We can add more later if needed to understand the purpose.

@johnbelamaric
Copy link
Contributor Author

I believe this is complete now, and ready for review.

@johnbelamaric
Copy link
Contributor Author

Rebased

- `Ready` is set to True if the last reconciliation successfully produced an
up-to-date Draft.

The PackageVariant resource will also contain a `DownstreamTargets` field,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@natasha41575 is this an accurate description?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that is accurate.

![Figure Legend](packagevariant-legend.png)

users.
[^pvsimpl]: This document describes PackageVariantSet `v1alpha2`, which has not
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mortent I think we should just go ahead an implement the machinery for conversion webhooks. I am assuming they would run in the controller binary. We're likely to need them in the future. The actual roundtrip conversions in this case are pretty easy to do (the proposed v1alpha2 API is a superset of the v1alpha1 functionality).

Copy link
Contributor

@mortent mortent left a comment

Choose a reason for hiding this comment

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

Just a small nit, but looks good.

docs/design-docs/08-package-variant.md Outdated Show resolved Hide resolved
@johnbelamaric johnbelamaric merged commit 04d3a04 into kptdev:main Apr 25, 2023
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