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

Automatically update dependencies on elastic-package and package-spec #802

Closed
mtojek opened this issue Mar 24, 2021 · 10 comments
Closed

Automatically update dependencies on elastic-package and package-spec #802

mtojek opened this issue Mar 24, 2021 · 10 comments

Comments

@mtojek
Copy link
Contributor

mtojek commented Mar 24, 2021

I was thinking about enabling a similar idea to the renovate project: https://github.com/renovatebot/renovate

It would great to automate dependency update on elastic-package (when a new master version is available) and do similar actions for package-spec.

See renovate in action here: https://github.com/grpc-ecosystem/grpc-gateway/commits?author=renovate-bot

/cc @ycombinator @kuisathaverat

@ycombinator
Copy link
Contributor

I'm in favor of this idea. From what I can tell skimming through the Renovate project, it will file a PR automatically when a dependency needs updating.

Occasionally CI will fail on such PRs for us because some tests need updating. At other times, CI might be green but we might want to manually make some changes to the PR to use some features that were brought in by the updated dependency. In such cases will we able to do such manual intervention on the automatically-generated PR?

@mtojek
Copy link
Contributor Author

mtojek commented Mar 24, 2021

To make it simple I would base on a single rule - is dependency's update present? CI is happy? Let's merge it.

If we need to introduce more changes on top of that, we can do a follow-up PR or temporarily disable the renovate.

@ycombinator
Copy link
Contributor

To make it simple I would base on a single rule - is dependency's update present? CI is happy? Let's merge it.

If we need to introduce more changes on top of that, we can do a follow-up PR or temporarily disable the renovate.

Yes, this is the part I'm trying to understand: what will happen if CI is not happy on the automated PR. Do we merge it anyway and then make a follow up PR to fix CI? That means CI will be broken on master for some period of time which is not ideal.

Alternatively would it possible for us to manually update the automated PR with commits to make it green? That would be ideal IMO.

@mtojek
Copy link
Contributor Author

mtojek commented Mar 24, 2021

Yes, this is the part I'm trying to understand: what will happen if CI is not happy on the automated PR. Do we merge it anyway and then make a follow up PR to fix CI? That means CI will be broken on master for some period of time which is not ideal.

No, I think in this case, the "renovate" (any other automation) should open an issue for us that master is not healthy.

@ycombinator
Copy link
Contributor

ycombinator commented Mar 24, 2021

No, I think in this case, the "renovate" (any other automation) should open an issue for us that master is not healthy.

I guess what I'm trying to prevent is master getting into an unhealthy state to begin with, due to the dependency PR getting merged. It would be great if we could manually push some commits to the the automated dependency PR if CI is not green on it, so we never have to merge a red PR.

@mtojek
Copy link
Contributor Author

mtojek commented Mar 24, 2021

I think it's doable, it should be a matter of branch permissions.

EDIT:
We can always close the automatically open PR and open another one with missing parts.

@ycombinator
Copy link
Contributor

We can always close the automatically open PR and open another one with missing parts.

Good point. We can fallback to this manual approach in such (hopefully rare) cases.

@mtojek
Copy link
Contributor Author

mtojek commented Mar 30, 2021

@v1v @kuisathaverat Is it something you prefer to handle observability-wide or should we proceed internally here?

@v1v
Copy link
Member

v1v commented Mar 30, 2021

Thanks @mtojek for the heads up.

Regarding the dependency manager service, we enabled back in the days dependabot, some teams are using it. I don't know if dependabot was evaluated already. If you enable any other tooling, you might need to go through the legal/security validation. dependabot is already approved.

Important -> renovate is already approved and used in Kibana elastic/kibana#60422 . Therefore there is no need to go through the approval process.

In order to run in the CI, we need to add a configuration in our end in case renovate is enabled -> https://github.com/elastic/apm-pipeline-library/blob/c0edf5b8ea6a596638f6c36ca0a98bde5f4ad983/vars/githubPrCheckApproved.groovy#L103

@botelastic
Copy link

botelastic bot commented Sep 19, 2022

Hi! We just realized that we haven't looked into this issue in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Sep 19, 2022
@botelastic botelastic bot closed this as completed Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants