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

Improve dependency management #18925

Open
ivanvc opened this issue Nov 20, 2024 · 7 comments
Open

Improve dependency management #18925

ivanvc opened this issue Nov 20, 2024 · 7 comments
Labels
area/security area/tooling priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. type/feature

Comments

@ivanvc
Copy link
Member

ivanvc commented Nov 20, 2024

What would you like to be added?

The current dependency management process involves a lot of manual intervention. There have been some attempts at improving it, including updating the update_deps.sh script, trying out different dependabot configurations, and evaluating other tools besides dependabot.

I want to formalize the work and have a central place to discuss alternatives, pros, and cons.

Possible approaches

  1. Do nothing / keep dependency management as it is right now. As stated before, the current process requires a lot of manual intervention, and the toil is high. The time to create the weekly PR and merge it keeps sliding into later in the week. There are times in which by human error, the commits are not properly bumping the dependencies.
  2. Improve dependabot configuration (so far, I've tested the following options):
    1. Use the new directories property: The result of this is multiple pull requests for the same dependency across all of the submodules where it is present. Although we (@jmhbnz and I) thought this was going to be the silver bullet, in the end, it doesn't work as we expected. Enabling this will just add noise to our repository, as we still need to create a PR to bump dependencies manually. See the following example of go.opentelemetry.io/otel/trace in my fork: build(deps): bump go.opentelemetry.io/otel/trace from 1.30.0 to 1.32.0 in /tests ivanvc/etcd#410 build(deps): bump go.opentelemetry.io/otel/trace from 1.30.0 to 1.32.0 in /server ivanvc/etcd#406 build(deps): bump go.opentelemetry.io/otel/trace from 1.30.0 to 1.32.0 in /etcdutl ivanvc/etcd#390 build(deps): bump go.opentelemetry.io/otel/trace from 1.30.0 to 1.32.0 ivanvc/etcd#386.
    2. Use the directories property and groups. Although this looked promising, as it creates a single pull request with all the weekly dependencies, the issues that I find are: 1. it creates a single commit for the whole group (rather than one per dependency bumped), 2. it doesn't run go mod tidy, and it breaks our CI. See an example in my fork: build(deps): bump the weekly-updates group across 12 directories with 70 updates ivanvc/etcd#265 (in the files changed and any go.sum file, it's clear that it is not tidying them).
  3. Replace dependabot with renovatebot. I noticed that several projects on board for dependabot's directories beta feature moved to renovate. The immediate issue is that we would need a new GitHub application installed for our GitHub organization. This is not the silver bullet but may help decrease the effort. I tested some configurations in my fork:
    1. One pull request per dependency bump: This would be similar to what we have right now, but it bumps the dependency across all submodules (and tides them). This will be a minimal improvement, as we can merge and close these PRs without the manual step to generate a grouped PR. Refer to an example in my fork: chore(deps): update module golang.org/x/crypto to v0.29.0 - abandoned ivanvc/etcd#435
    2. Grouped pull request: This doesn't work as expected. It's similar to dependabot's approach (single commit for all bumps), although the community seems interested in having multiple commits (Support multiple commits per branch/PR renovatebot/renovate#7288). An example of this configuration in my fork: fix(deps): update gomod ivanvc/etcd#442
  4. A wild idea would be to write our own automation, but I think it would be less than ideal as we'd need to maintain another project.

I lean towards 3i (renovate + one pull request per dependency), which would be one small step to improve the process. But I want to open the topic for discussion.

cc. @ArkaSaha30

Why is this needed?

To decrease the toil of keeping dependencies up to date.

@ivanvc ivanvc added type/feature area/tooling priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Nov 20, 2024
@jmhbnz
Copy link
Member

jmhbnz commented Nov 20, 2024

Thanks for all your work experimenting and writing up the findings @ivanvc!

I am in favor of moving away from needing to manually raise a pull request every week to bump dependencies to instead be able to merge automated pr's raised by renovate, so option 3.

This will be less toil for the community to manage, and one less bespoke project specific process for new contributors to learn.

@ahrtr
Copy link
Member

ahrtr commented Nov 20, 2024

I lean towards 3i (renovate + one pull request per dependency), which would be one small step to improve the process.

Questions:

  • Once we merge each PR, we also need to wait for other PRs to automatically rebase and green again? So we have to approve & merge the PRs one by one?
  • We still need to manually close PRs which bump pure indirect dependencies?

@ivanvc
Copy link
Member Author

ivanvc commented Nov 20, 2024

Questions:

* Once we merge each PR, we also need to wait for other PRs to automatically rebase and green again? So we have to approve & merge the PRs one by one?

That's a good question. I think so, because they will be editing the same files. This may slow down the merging process of the whole week's dependencies. However, we can fine tune how often the bot opens the pull requests (i.e., you can specify how many per hour it should open).

* We still need to manually close PRs which bump pure indirect dependencies?

Yes, and you just made me realize that because of how we do the dependency management, we may not ever be able to do a single pull request (if they ever support multiple commits in a single one), because we would want to remove some dependencies that are purely indirect, but we want to keep the ones that are direct in some submodules but indirect in others.

@jmhbnz
Copy link
Member

jmhbnz commented Nov 20, 2024

Yes, and you just made me realize that because of how we do the dependency management, we may not ever be able to do a single pull request (if they ever support multiple commits in a single one), because we would want to remove some dependencies that are purely indirect, but we want to keep the ones that are direct in some submodules but indirect in others.

One idea we could add all purely indirect dependencies to the renovate ignore list. Provided renovate would still bump them when a direct dependency requires it which I would think it would as it runs go mod tidy.

@ahrtr
Copy link
Member

ahrtr commented Nov 21, 2024

This may slow down the merging process of the whole week's dependencies. However, we can fine tune how often the bot opens the pull requests (i.e., you can specify how many per hour it should open).

OK. Since usually we just have several dependency bumping (i.e. around 3) each time, so it might be fine. Regarding the frequency, either Weekly or bi-weekly is OK to me.

One idea we could add all purely indirect dependencies to the renovate ignore list.

It will introduce extra maintenance effort, because the list may change over time.

@ivanvc
Copy link
Member Author

ivanvc commented Nov 21, 2024

It will introduce extra maintenance effort, because the list may change over time.

It may need some experimentation, but maybe through commands we could cherry pick some dependencies out of the grouped PR, following James' idea. I'll report back.

@ivanvc
Copy link
Member Author

ivanvc commented Dec 3, 2024

I opened a dependabot-core issue (dependabot/dependabot-core#11046) to track the lack of running go mod tidy.

Unfortunately, renovate doesn't have an interactive bot interface in the PR like dependabot. I was also testing dependabot trying to ignore a single dependency in a grouped PR, but it doesn't seem to work (ivanvc#447)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security area/tooling priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. type/feature
Development

No branches or pull requests

3 participants