argoproj / gitops-engine

Democratizing GitOps
https://pkg.go.dev/github.com/argoproj/gitops-engine?tab=subdirectories
Apache License 2.0
1.7k stars 264 forks source link

Bump kubernetes to 1.28.4 #556

Closed farodin91 closed 6 months ago

majolo commented 11 months ago

This would be useful for us, I was also about to look into upgrading! Is there any reason you chose not to upgrade to v1.28.4 of k8s.io/kubernetes and go 1.21?

codecov[bot] commented 11 months ago

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (4a5648e) 54.70% compared to head (3580f83) 54.78%. Report is 1 commits behind head on master.

:exclamation: Current head 3580f83 differs from pull request most recent head dc1d73d. Consider uploading reports for the commit dc1d73d to get more accurate results

Files Patch % Lines
pkg/sync/sync_context.go 79.16% 3 Missing and 2 partials :warning:
pkg/utils/kube/resource_ops.go 0.00% 4 Missing :warning:
pkg/utils/kube/kube.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #556 +/- ## ========================================== + Coverage 54.70% 54.78% +0.08% ========================================== Files 41 41 Lines 4645 4636 -9 ========================================== - Hits 2541 2540 -1 + Misses 1908 1899 -9 - Partials 196 197 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

farodin91 commented 11 months ago

This would be useful for us, I was also about to look into upgrading! Is there any reason you chose not to upgrade to v1.28.4 of k8s.io/kubernetes and go 1.21?

Updated.

sonarcloud[bot] commented 11 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

crenshaw-dev commented 11 months ago

Thanks so much for picking this up, @majolo! I'm going to wait until we cut Argo CD 2.10 before tackling this, since we have some other high-priority changes we want to get in.

farodin91 commented 11 months ago

@crenshaw-dev Sounds great. Do you have a time range? We are blocked by this as we can't upgrade to a newer version of kubebuilder.

crenshaw-dev commented 11 months ago

@farodin91 the earliest will be > next Monday.

I'll go ahead and give a quick first-pass review so we can knock out any basic concerns.

majolo commented 11 months ago

Thanks @crenshaw-dev could this not be cut first and then included in argo-cd 2.10? It's actually argo-cd that we use (which in turn imports this library) which is causing us the issues.

crenshaw-dev commented 11 months ago

@majolo theoretically, yes. But practically speaking, no. Before this can be merged, a similar PR will need to be made on the argo-cd repo and reviewed/tested. With those two PRs together, I just don't think I'll have time to review/merge before we cut RC1 Monday.

dfroberg commented 11 months ago

Any chance of getting this one bumped together with this one: https://github.com/argoproj/gitops-engine/issues/558

majolo commented 10 months ago

@crenshaw-dev any update on this one? Thanks!

asaf-erlich commented 10 months ago

Hello 👋 , our team owns an internal tool that interacts with both argo cd and k8s. As part of the recent upgrade a different team is making to all our k8s clusters to 1.27+ I'm trying to upgrade our tool's internal libraries. When doing so the gitops-engine dependency, even if I bump it to the sha in main/master is not able to compile and is blocking the upgrade.

This PR's code works perfectly with no compile errors (I checked out the forked code locally). I would love to know a rough timeline of when this could get merged? Alternatively if you're willing to push a tag or something so my go.mod file could depend on it. This would be greatly appreciated 🙏

crenshaw-dev commented 10 months ago

@asaf-erlich we can start working on getting this merged now.

Would you mind rebasing?

Could you also open a PR on argo-cd pointing to the latest commit of your fork/branch? Here's an example of what that change looks like. https://github.com/argoproj/argo-cd/pull/16858/commits/90ce09a75ee7136d08bc11d9c89018e2353d6652

That PR will let us run Argo CD's tests and catch any issues which weren't revealed by the gitops-engine tests.

crenshaw-dev commented 10 months ago

Looks like we could actually go to 1.28.6 or to 1.29.1 at this point.

asaf-erlich commented 10 months ago

@crenshaw-dev I'm not the original author of the PR, @farodin91 is. I'm happy to attempt those things it will help but would need to create a new PR to do so. I'd like to give @farodin91 at least a day or two to respond and if they cannot do it I can try to do so.

farodin91 commented 10 months ago

@crenshaw-dev I'm not the original author of the PR, @farodin91 is. I'm happy to attempt those things it will help but would need to create a new PR to do so. I'd like to give @farodin91 at least a day or two to respond and if they cannot do it I can try to do so.

Go for it. I'm on vacation and next in this week or next week.

asaf-erlich commented 10 months ago

Go for it. I'm on vacation and next in this week or next week.

I'll give it a try

asaf-erlich commented 10 months ago

Here is the rebased PR: https://github.com/argoproj/gitops-engine/pull/565

asaf-erlich commented 10 months ago

Could you also open a PR on argo-cd pointing to the latest commit of your fork/branch? Here's an example of what that change looks like. https://github.com/argoproj/argo-cd/commit/90ce09a75ee7136d08bc11d9c89018e2353d6652

That PR will let us run Argo CD's tests and catch any issues which weren't revealed by the gitops-engine tests.

Looking at the PR and at https://stackoverflow.com/questions/72312460/how-to-replace-a-go-module-with-a-major-version-to-a-fork-master I will need to edit my own fork's go.mod file, at least temporarily, to declare a different name.

Edit: maybe I'll just push that rename to a different branch and create a tag instead to avoid messing with my PR

asaf-erlich commented 10 months ago

My first attempt at the test PR (left it in draft state): https://github.com/argoproj/argo-cd/pull/16967

recharte commented 7 months ago

Hi @asaf-erlich and @crenshaw-dev, would you by any chance be able to drive this effort to completion?

I'm in a similar situation as the one described by @asaf-erlich and would greatly benefit from this upgrade. I would even advocate for going a bit further and bumping to 1.29.x, if possible.

asaf-erlich commented 7 months ago

Unfortunately I don't have the time to make the required changes.

crenshaw-dev commented 7 months ago

Focusing effort on this PR: https://github.com/argoproj/gitops-engine/pull/566

blakepettersson commented 6 months ago

Superceded by #566.