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

Upgrade Go, controller-runtime and related dependencies #1175

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

levan-m
Copy link
Contributor

@levan-m levan-m commented May 9, 2024

What does this PR do?

See CECO-1114 for details about specific changes.

  • EDS
    v0.9.0-rc.2 -> v0.10.0-rc.4
  • sigs.k8s.io/controller-runtime
    v0.12.2 -> v0.16.0
  • k8s.io/api, k8s.io/apimachinery, k8s.io/client-go
    0.24.2 -> v0.28.9
  • go.uber.org/zap
    v1.19.1 -> 0.25.0
  • go
    1.19 -> 1.21

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

Write there any instructions and details you may have to test your PR.

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: bug, enhancement, refactoring, documentation, tooling, dependencies

@levan-m levan-m force-pushed the levan-m/controller-runtime-update branch from ed9fbd4 to b7c6e6b Compare May 9, 2024 21:20
…`make docker-build` fails on EDS checksum mismatch
@levan-m levan-m force-pushed the levan-m/controller-runtime-update branch from b7c6e6b to 24c0436 Compare May 10, 2024 22:03
@levan-m levan-m added this to the v1.7.0 milestone May 11, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 11, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 59.58%. Comparing base (6b0c7bd) to head (514993d).
Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1175      +/-   ##
==========================================
+ Coverage   59.55%   59.58%   +0.02%     
==========================================
  Files         174      174              
  Lines       21559    21622      +63     
==========================================
+ Hits        12839    12883      +44     
- Misses       7941     7958      +17     
- Partials      779      781       +2     
Flag Coverage Δ
unittests 59.58% <90.90%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
controllers/datadogagent_controller.go 71.92% <100.00%> (+1.02%) ⬆️
pkg/plugin/common/client.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b0c7bd...514993d. Read the comment docs.

@levan-m levan-m force-pushed the levan-m/controller-runtime-update branch 2 times, most recently from 514993d to abebac8 Compare May 14, 2024 18:38
@levan-m levan-m force-pushed the levan-m/controller-runtime-update branch from abebac8 to dd7b1c9 Compare May 14, 2024 18:44
@levan-m levan-m marked this pull request as ready for review May 14, 2024 18:57
@levan-m levan-m requested review from a team as code owners May 14, 2024 18:57
// },
// ObjectMeta: metav1.ObjectMeta{
// Name: "test monitor",
// // https://github.com/kubernetes-sigs/controller-runtime/commit/7a66d580c0c53504f5b509b45e9300cc18a1cc30#diff-20ecedbf30721c01c33fb67d911da11c277e29990497a600d20cb0ec7215affdR683-R686
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI @celenechang, I couldn't find a quick workaround. Let me know if this is a blocker.

@levan-m levan-m changed the title Update controller-runtime to 0.15.0 Upgrade Go, controller-runtime and related dependencies May 14, 2024
Copy link
Contributor

@maycmlee maycmlee left a comment

Choose a reason for hiding this comment

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

A couple of suggestions

@@ -340,7 +342,7 @@ In the table, `spec.override.nodeAgent.image.name` and `spec.override.nodeAgent.
| [key].securityContext.seLinuxOptions.user | User is a SELinux user label that applies to the container. |
| [key].securityContext.seccompProfile.localhostProfile | localhostProfile indicates a profile defined in a file on the node should be used. The profile must be preconfigured on the node to work. Must be a descending path, relative to the kubelet's configured seccomp profile location. Must only be set if type is "Localhost". |
| [key].securityContext.seccompProfile.type | type indicates which kind of seccomp profile will be applied. Valid options are: Localhost - a profile defined in a file on the node should be used. RuntimeDefault - the container runtime default profile should be used. Unconfined - no profile should be applied. |
| [key].securityContext.supplementalGroups | A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows. |
| [key].securityContext.supplementalGroups | A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| [key].securityContext.supplementalGroups | A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows. |
| [key].securityContext.supplementalGroups | A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the UID of the container process. If unspecified, no additional groups are added to any container. **Note**: Group memberships defined in the container image for the UID of the container process are still effective, even if they are not included in this list. This field cannot be set when spec.os.name is Windows. |

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of these nested types get docs from the dependent packages and we can't really modify it. This should come from here
https://github.com/kubernetes/api/blob/02024d286e48d744890214fa862cb7ec9df865af/core/v1/types.go#L3785-L3793

@levan-m levan-m force-pushed the levan-m/controller-runtime-update branch from 61b82f3 to 30616a8 Compare May 22, 2024 15:35
@levan-m levan-m modified the milestones: v1.7.0, v1.8.0 May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants