Skip to content

⚠️ Update to k8s v0.30.1 - #912

Merged
varshaprasad96 merged 1 commit into
operator-framework:mainfrom
tmshort:update-k8s
Jun 6, 2024
Merged

⚠️ Update to k8s v0.30.1#912
varshaprasad96 merged 1 commit into
operator-framework:mainfrom
tmshort:update-k8s

Conversation

@tmshort

Copy link
Copy Markdown
Contributor

Also requires update to controller-runtime

This should clear the way for other updates (that require k8s)

Description

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Also requires update to controller-runtime
Signed-off-by: Todd Short <tshort@redhat.com>
@tmshort
tmshort requested a review from a team as a code ownerJune 5, 2024 15:49
@netlify

netlifyBot commented Jun 5, 2024

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commit5d31895
🔍 Latest deploy loghttps://app.netlify.com/sites/olmv1/deploys/666088f5f26f9a0008a525ca
😎 Deploy Previewhttps://deploy-preview-912--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tmshorttmshort mentioned this pull request Jun 5, 2024
4 tasks
Comment on lines -335 to +347
uMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
if err != nil {
setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err))
return ctrl.Result{}, err
}

unstructuredObj := &unstructured.Unstructured{Object: uMap}
if err := func() error {
r.dynamicWatchMutex.Lock()
defer r.dynamicWatchMutex.Unlock()

_, isWatched := r.dynamicWatchGVKs[unstructuredObj.GroupVersionKind()]
_, isWatched := r.dynamicWatchGVKs[obj.GetObjectKind().GroupVersionKind()]
if !isWatched {
if err := r.controller.Watch(
source.Kind(r.cache, unstructuredObj),
crhandler.EnqueueRequestForOwner(r.Scheme(), r.RESTMapper(), ext, crhandler.OnlyControllerOwner()),
helmpredicate.DependentPredicateFuncs()); err != nil {
source.Kind(r.cache,
obj,
crhandler.EnqueueRequestForOwner(r.Scheme(), r.RESTMapper(), ext, crhandler.OnlyControllerOwner()),
helmpredicate.DependentPredicateFuncs())); err != nil {
return err
}
r.dynamicWatchGVKs[unstructuredObj.GroupVersionKind()] = struct{}{}
r.dynamicWatchGVKs[obj.GetObjectKind().GroupVersionKind()] = struct{}{}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@tmshorttmshortJun 5, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Not sure which fixup is better, but I suggest we do the k8s update here in #912, and focus #905 to be just rukpak.

@varshaprasad96varshaprasad96Jun 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we use the unstructured one for now and have this PR merged? #905 (comment)

Just curious if directly setting watches from typed objects would break anything, which is why suggesting the other option.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 @varshaprasad96. I don't know for sure that using a structured type will break things, but I have at least some confidence that it might. So I think we should stick with the unstructured type for now.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

So, I tested this, with help from @joelanford, and it appears to work.

@tmshorttmshort changed the title ✨ Update to k8s v0.30.1⚠️ Update to k8s v0.30.1Jun 6, 2024

@varshaprasad96varshaprasad96 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2024
@varshaprasad96
varshaprasad96 added this pull request to the merge queue Jun 6, 2024
Merged via the queue into operator-framework:main with commit 35e5087Jun 6, 2024
@tmshort
tmshort deleted the update-k8s branch June 6, 2024 14:15
@skattojuskattoju mentioned this pull request Sep 25, 2024
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtmIndicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tmshort@joelanford@varshaprasad96