Implement lifecycle CRD #1628
Open
Implement lifecycle CRD #1628
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
…r/cluster builder reconcilers Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
…ic CRDs Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
| RegistryClient: ®istry.Client{}, | ||
| KpackVersion: cmd.Identifer, | ||
| LifecycleProvider: lifecycleProvider, | ||
| KeychainFactory: keychainFactory, |
There was a problem hiding this comment.
Unrelated change, this seems no longer used
| @@ -34,7 +34,9 @@ type Builder struct { | |||
| type BuilderSpec struct { | |||
| Tag string `json:"tag,omitempty"` | |||
| Stack corev1.ObjectReference `json:"stack,omitempty"` | |||
| Store corev1.ObjectReference `json:"store,omitempty"` | |||
| // TODO: confirm, should `json:"lifecycle,...` actually be `json:"clusterlifecycle,...`? | |||
There was a problem hiding this comment.
I think...no? Since that is not the case for stack?
Comment on lines
43
to
50
| type ResolvedClusterLifecycle struct { | ||
| Id string `json:"id,omitempty"` // TODO: confirm, should this be LatestImage? | ||
| Version string `json:"version,omitempty"` | ||
|
|
||
| // Deprecated: Use `LifecycleAPIs` instead | ||
| API LifecycleAPI `json:"api,omitempty"` | ||
| APIs LifecycleAPIs `json:"apis,omitempty"` | ||
| } |
There was a problem hiding this comment.
Feedback on the structure of this struct would be appreciated
| ) | ||
|
|
||
| func (cl *ClusterLifecycle) SetDefaults(context.Context) { | ||
| // TODO: confirm, should we add something here? |
There was a problem hiding this comment.
Is this where we specify the image to use if the lifecycle CRD is not configured?
| @@ -1063,24 +1087,6 @@ func (i *layerIteratorTester) testNextLayer(name string, test func(index int)) { | |||
| *i++ | |||
| } | |||
|
|
|||
| func layerToRemoteBuildpack(bpLayer buildpackLayer, layer *fakeLayer, secretRef registry.SecretRef) K8sRemoteBuildpack { | |||
There was a problem hiding this comment.
This was unused
| @@ -173,6 +181,17 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Bui | |||
| }, | |||
| }, builder.NamespacedName()) | |||
|
|
|||
| c.Tracker.Track(reconciler.Key{ | |||
| NamespacedName: types.NamespacedName{ | |||
| Name: builder.Spec.Lifecycle.Name, // TODO: confirm this is what we want | |||
There was a problem hiding this comment.
Right? Is there a better name for "Name" ...like maybe "ImageName" or "ImageTag"?
| @@ -224,6 +243,11 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Bui | |||
| return buildapi.BuilderRecord{}, errors.Errorf("Error: clusterstack '%s' is not ready", clusterStack.Name) | |||
| } | |||
|
|
|||
| clusterLifecycle, err := c.ClusterLifecycleLister.Get(builder.Spec.Lifecycle.Name) // TODO: confirm this is what we want | |||
There was a problem hiding this comment.
Same Q as above
| @@ -0,0 +1,145 @@ | |||
| package clusterlifecycle | |||
There was a problem hiding this comment.
I basically just copied the reconciler for clusterstack
Signed-off-by: Natalie Arellano <[email protected]>
…e image digest We don't have the lifecycle image digest when inspecting the build status Signed-off-by: Natalie Arellano <[email protected]>
it will never be ready Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Commit is not going to work because we can't get this information from the lifecycle image labels, it is only available after a build from the application image labels Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
|
Small update: I was able to create a clusterlifecycle and observe a new build get triggered when the lifecycle version updated. Still working on getting the e2e tests working locally (having credential issues) and there are a few outstanding TODOs that I can resolve on my own, but this is ready for some feedback! 🙏🏼 |
pkg/cnb/create_builder.go
Outdated
| @@ -50,6 +45,10 @@ func (r *RemoteBuilderCreator) CreateBuilder( | |||
| if err != nil { | |||
| return buildapi.BuilderRecord{}, err | |||
| } | |||
| lifecycleImage, _, err := r.RegistryClient.Fetch(lifecycleKeychain, clusterLifecycle.Spec.Image) // TODO: should there be a "latest image" on the status? | |||
There was a problem hiding this comment.
That way we could at least know the exact digest that was used, even if we don't use it anywhere?
Signed-off-by: Natalie Arellano <[email protected]>
natalieparellano
changed the title
This was referenced Jun 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement lifecycle CRD (RFC PR)
I am still testing this out locally to make sure everything threads together, but it might be ready for some initial feedback on the direction.Marking as ready in the interest of getting feedback.