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 k8s dependencies to 1.31.0 #3234

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

swiatekm
Copy link
Contributor

Also bump controller-runtime to 0.19.0. This also required upgrading controller-tools to 0.16.1.

Supersedes #3233.

@swiatekm swiatekm added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Aug 21, 2024
@swiatekm swiatekm requested a review from a team as a code owner August 21, 2024 12:00
@swiatekm
Copy link
Contributor Author

I don't really understand what's going on with the managementState field here. The field is marked as required, so the change to the CRDs is correct, but I'm not sure why it wasn't the case in the first place. The E2E test failure is a bit confusing. kubectl replace doesn't apply defaults, so it makes sense that it would fail validation, but a plain apply also doesn't work, for reasons which are unclear to me.

Also bump controller-runtime to 0.19.0.
Comment on lines -155 to -179
verbs:
- get
- patch
- update
- apiGroups:
- opentelemetry.io
resources:
- opentelemetrycollectors
verbs:
- get
- list
- patch
- update
- watch
- apiGroups:
- opentelemetry.io
resources:
- opentelemetrycollectors/finalizers
verbs:
- get
- patch
- update
- apiGroups:
- opentelemetry.io
resources:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are those not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section was merged with the one for Instrumentations. I guess the new version of controller-tools is better at arranging RBAC rules.

@@ -3284,6 +3313,8 @@ spec:
type: object
type: array
x-kubernetes-list-type: atomic
required:
- managementState
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that users will always need to provide the .spec.managementState field when creating an OpenTelemetryCollector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, managementState has a default, and that applies if it's not set by the user. Otherwise, all the E2E tests would fail.

Worth noting that I had to change one test which did a kubectl replace. That command does not apply defaults, and in that case you do need to set managementState manually. I just changed it into an apply instead.

@jaronoff97 jaronoff97 merged commit 2866a95 into open-telemetry:main Aug 22, 2024
33 checks passed
@swiatekm swiatekm deleted the deps/bump-k8s-dependencies branch August 22, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants