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.

Sign up for GitHub

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

Jump to bottom

Update gardener/gardener to v1.86.0 #748

Merged
merged 15 commits into from
Apr 16, 2024
Merged

Update gardener/gardener to v1.86.0 #748

merged 15 commits into from
Apr 16, 2024

Conversation

shreyas-s-rao
Copy link
Contributor

@shreyas-s-rao shreyas-s-rao commented Jan 7, 2024 •
edited
Loading

How to categorize this PR?

/area dev-productivity
/kind cleanup

What this PR does / why we need it:
This PR updates the gardener/gardener dependency to v1.86.0, bringing in multiple other changes:

  • Remove vendor directory, in line with gardener, similar to Refactoring: Remove vendoring from project gardener#8775
  • Bump dependencies, required for g/g v1.86.0
  • Update paths in Makefile, CI scripts and hack scripts to remove usage of vendor dir for g/g hack scripts, and find the g/g dir in mod cache instead
  • Remove -mod=vendor from builds, runs and test runs
  • Run make tidy, which pulls in new CI hack scripts and issue/PR templates
  • Adapt and run make generate
  • Remove unused chart renderer code from g/g, which was deprecated and now removed in the latest g/g version
  • Adapt manager and controller registration code to new controller-runtime version, both in main code and integration test utils
  • Replace github.com/golang/mock/gomock with go.uber.org/mock/gomock, as stated in https://github.com/golang/mock
  • Adapt etcdcopybackupstask deletion unit tests to new controller-runtime fake client behavior. There is a change in behavior of fake client handling of deletion timestamps, as well as handling updates to status subresource of CRDs, due to which the tests needed to be adapted
  • Downgrade kustomize from v5.0.0 to v4.5.7 due to this bug, which we and many other users used as a feature. [Backlog] Upgrade kustomize to v5.0.0+ #765 details out the issue and a recommendation to properly fix this in the long-term, but this PR should go ahead with the checked in fix.
  • Update github issue templates with new ones from g/g.

Which issue(s) this PR fixes:
Fixes #734

Special notes for your reviewer:
/cc @afritzler @shafeeqes
/invite @seshachalam-yv @ishan16696 @unmarshall

It's easier to review this PR commit by commit, rather than all files as once, for two reasons: the commits are well-structured and segregated by individual purposes, and the removal of vendor dir causes a large diff, making it difficult for github to render the diff page. You may also review the set of commits omitting the first commit which removes the vendor dir.

Release note:

Vendor directory has now been removed from the project. Please run `make tidy` to pull dependencies into go mod cache initially, and whenever required.

@shreyas-s-rao shreyas-s-rao added this to the v0.22.0 milestone Jan 7, 2024
@shreyas-s-rao shreyas-s-rao requested a review from a team as a code owner January 7, 2024 15:57
@gardener-robot gardener-robot added the needs/review Needs review label Jan 7, 2024
@gardener-robot gardener-robot added area/dev-productivity Developer productivity related (how to improve development) kind/cleanup Something that is not needed anymore and can be cleaned up labels Jan 7, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 7, 2024
@gardener-robot gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Jan 7, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 7, 2024
@shreyas-s-rao shreyas-s-rao modified the milestones: v0.22.0, v0.23.0 Jan 9, 2024
@shreyas-s-rao
Copy link
Contributor Author

/retest

shafeeqes
shafeeqes reviewed Jan 9, 2024
View reviewed changes
Copy link
Contributor

@shafeeqes shafeeqes left a comment

Choose a reason for hiding this comment

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

Thanks for the well-structured PR. Only minor comments from my side.

Makefile Outdated Show resolved Hide resolved
controllers/etcdcopybackupstask/reconciler_test.go Show resolved Hide resolved
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 9, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 9, 2024
@shreyas-s-rao
Copy link
Contributor Author

@shafeeqes thanks for your review. I have addressed both comments. PTAL

@gardener-robot
Copy link

@unmarshall, @ishan16696, @seshachalam-yv You have pull request review open invite, please check

@shreyas-s-rao
Copy link
Contributor Author

/retest

@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 16, 2024
@shreyas-s-rao
Copy link
Contributor Author

Something seems wrong with the prow jobs. e2e tests are passing when run locally:
druid-e2e.log

@shreyas-s-rao
Copy link
Contributor Author

/retest

@shreyas-s-rao shreyas-s-rao self-assigned this Feb 1, 2024
@shreyas-s-rao shreyas-s-rao mentioned this pull request Feb 2, 2024
Closed
@shreyas-s-rao
Copy link
Contributor Author

/retest

@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 16, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 16, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 16, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 16, 2024
@unmarshall unmarshall merged commit 637da10 into gardener:master Apr 16, 2024
8 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Apr 16, 2024
@shreyas-s-rao shreyas-s-rao deleted the dep/gardener-v1.86.0 branch April 16, 2024 10:52
@renormalize renormalize mentioned this pull request Apr 17, 2024
Closed
renormalize added a commit to renormalize/etcd-druid that referenced this pull request Apr 19, 2024
@renormalize
Add the vendor directory to .gitignore
8f6b310 
* gardener#748 removed the `vendor` directory from `etcd-druid` and makes the
  project rely on the module cache set up by the `go` tool for finding
  dependencies.

* However, there are use cases where for better developer experience, it
  would make sense to have the dependencies reside in the same project
  folder as `etcd-druid`, so that tokens can be searched for, using
  project wide search in any text editor. Results can be fetched from
  the dependencies, without having to rely on the LSP.
  For example, this would make looking at all usages of a constant that
  is defined by a dependency easier.

* The developer can simply run `go mod vendor` without it affecting the
  modified build process which doesn't use `vendor`, as defined in gardener#748.
@renormalize renormalize mentioned this pull request Apr 19, 2024
Merged
@shreyas-s-rao shreyas-s-rao modified the milestones: v0.24.0, v0.23.0 Apr 24, 2024
@shreyas-s-rao shreyas-s-rao mentioned this pull request Apr 25, 2024
Closed
renormalize pushed a commit that referenced this pull request Jun 26, 2024
@unmarshall
Fixes reuse compliance (#816)
eb67904 
* Fixes REUSE compliance by removing references to dependencies in `.reuse/dep5` which were previously vendor-ed in. (The vendor directory was removed in #748)

* Removes the unused licenses from the `LICENSES` directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shafeeqes shafeeqes shafeeqes left review comments

@unmarshall unmarshall unmarshall approved these changes

@ishan16696 ishan16696 Awaiting requested review from ishan16696

@seshachalam-yv seshachalam-yv Awaiting requested review from seshachalam-yv

Assignees

@shreyas-s-rao shreyas-s-rao

@ishan16696 ishan16696

Labels
area/dev-productivity Developer productivity related (how to improve development) kind/cleanup Something that is not needed anymore and can be cleaned up needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/rebase Needs git rebase needs/review Needs review needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Milestone
v0.23.0
Development

Successfully merging this pull request may close these issues.

[Feature] Bump Gardener dependency
8 participants
@shreyas-s-rao @gardener-robot @unmarshall @shafeeqes @gardener-robot-ci-1 @ishan16696 @gardener-robot-ci-2 @gardener-robot-ci-3