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

🌱 Bump helm to 3.14.1 #823

Merged
merged 1 commit into from Feb 22, 2024
Merged

Conversation

kevinrizza
Copy link
Member

@kevinrizza kevinrizza commented Feb 15, 2024

Also bumps kubernetes to 1.29

@kevinrizza kevinrizza requested a review from a team as a code owner February 15, 2024 15:59
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f8811ca) 35.27% compared to head (7d9574e) 35.27%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #823   +/-   ##
=======================================
  Coverage   35.27%   35.27%           
=======================================
  Files           9        9           
  Lines         808      808           
=======================================
  Hits          285      285           
  Misses        479      479           
  Partials       44       44           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

go.mod Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
@m1kola
Copy link
Member

m1kola commented Feb 19, 2024

@kevinrizza I haven't looked into this properly, but I have a feeling that the e2e failures are legit. Most likely we assert (for whatever reason) error messages which got changed.

@m1kola m1kola marked this pull request as draft February 21, 2024 15:46
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2024
Also bumps kubernetes dependencies to 1.29

Signed-off-by: kevinrizza <krizza@redhat.com>
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@m1kola m1kola changed the title Bump helm to 3.14.1 🌱 Bump helm to 3.14.1 Feb 21, 2024
@m1kola
Copy link
Member

m1kola commented Feb 21, 2024

I'm taking over this PR (with Kevin's permissions). I updated the branch and I think we should be good now.

@m1kola m1kola marked this pull request as ready for review February 21, 2024 16:52
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2024
Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

The changes look good, and all the tests pass.
/lgtm
/approve.

Just a small question regarding the change in error.

@@ -1337,7 +1337,7 @@ var _ = Describe("plain provisioner bundle", func() {
WithTransform(func(c *metav1.Condition) string { return c.Type }, Equal(rukpakv1alpha2.TypeInstalled)),
WithTransform(func(c *metav1.Condition) metav1.ConditionStatus { return c.Status }, Equal(metav1.ConditionFalse)),
WithTransform(func(c *metav1.Condition) string { return c.Reason }, Equal(rukpakv1alpha2.ReasonInstallFailed)),
WithTransform(func(c *metav1.Condition) string { return c.Message }, ContainSubstring(`the server could not find the requested resource`)),
WithTransform(func(c *metav1.Condition) string { return c.Message }, ContainSubstring(`no matches for kind "CatalogSource" in version "operators.coreos.com/v1alpha1"`)),
Copy link
Member

Choose a reason for hiding this comment

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

Though no matches for the kind and could not find the resources return the same HTTP error code, I believe the former means that the CRD is not installed available in the cluster (API server does not know the definition), whereas the latter indicates that the specific resource endpoint does not exist. Was there a change in k8s because of which this change was needed? Just curious.

Copy link
Member

Choose a reason for hiding this comment

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

@varshaprasad96 I believe that both errors were about missing CRDs originally. I think so because:

  • Full text of original error suggests this (complaining about missing group version, see examples below)
  • Test data for these two cases does not have these CRDs, but has CRs.

There was a number of breaking changes in controller-runtime@v0.17.0. I think most likely this is due to kubernetes-sigs/controller-runtime#2571.


Original errors:

error while running post render on files: postrenderer[0] (*client.ownerPostRenderer) failed: unable to recognize "": failed to get API group resources: unable to retrieve the complete list of server APIs: operators.coreos.com/v1: the server could not find the requested resource
error while running post render on files: postrenderer[0] (*client.ownerPostRenderer) failed: unable to recognize "": failed to get API group resources: unable to retrieve the complete list of server APIs: operators.coreos.com/v1alpha1: the server could not find the requested resource

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2024
@m1kola m1kola added this pull request to the merge queue Feb 22, 2024
Merged via the queue into operator-framework:main with commit e1fceb0 Feb 22, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants