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

Create a job to update common files across repos #1627

Closed
geeknoid opened this issue Aug 8, 2019 · 16 comments · Fixed by #2363
Closed

Create a job to update common files across repos #1627

geeknoid opened this issue Aug 8, 2019 · 16 comments · Fixed by #2363
Assignees

Comments

@geeknoid
Copy link
Contributor

geeknoid commented Aug 8, 2019

Istio has the istio/common-files repo which contains files which should appear in every development repo in the org. When a push occurs in istio/common-files, we want a job to iterate through all repos in the org. For each repo, check out the repo and run "make updatecommon" and create a PR to push all changes.

@geeknoid geeknoid self-assigned this Aug 8, 2019
@howardjohn
Copy link
Member

Related to istio/istio#16381

@istio-policy-bot istio-policy-bot added the lifecycle/needs-triage Indicates a new PR or issue needs to be triaged label Oct 30, 2019
@clarketm
Copy link
Member

clarketm commented Nov 7, 2019

@geeknoid - based on our discussion, I will take this off your plate (unless it is already in progress).

@clarketm clarketm self-assigned this Nov 7, 2019
@geeknoid geeknoid removed their assignment Nov 7, 2019
@geeknoid
Copy link
Contributor Author

geeknoid commented Nov 7, 2019

Excellent, thanks.

@jasonwzm FYI

@istio-policy-bot istio-policy-bot removed the lifecycle/needs-triage Indicates a new PR or issue needs to be triaged label Nov 9, 2019
@clarketm
Copy link
Member

clarketm commented Nov 11, 2019

/assign @Biwwie

As discussed, the rough process should be something like:

Create postsubmit job(s) with 2 possible triggers:

  1. https://github.com/istio/tools/tree/master/docker/build-tools dir changes
  2. https://github.com/istio/common-files repo changes

With the following actions:

  1. If a change to build-tools, then update the image in all relevant jobs (e.g.
    image: gcr.io/istio-testing/build-tools:master-2019-11-08T17-21-18
    )
  2. If a change to common files, propagate new files to repo consumers (e.g. istio/istio, istio/test-infra)
    1. If and only if the apply does not break current presubmits
  3. Create a PR for the above changes.

Aside:
If we can simplify this flow with actual git submodule(s), then now would be an ideal time to make that change.

@istio-testing
Copy link
Collaborator

@clarketm: GitHub didn't allow me to assign the following users: Biwwie.

Note that only istio members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @Biwwie

As discussed, the rough process should be something like:

Some postsubmit job(s) with 2 possible triggers:

  1. https://github.com/istio/tools/tree/master/docker/build-tools dir changes
  2. https://github.com/istio/common-files repo changes

With the following actions:

  1. If the trigger was a change to build-tools, then update the image in all relevant jobs (e.g.
    image: gcr.io/istio-testing/build-tools:master-2019-11-08T17-21-18
    )
  2. Applying the new common files to repo consumers (e.g. istio/istio, istio/test-infra)
    1. If and only if the apply does not break current presubmits
  3. Create a PR for the above changes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@howardjohn
Copy link
Member

More info about a broader issue: istio/istio#16381

@clarketm
Copy link
Member

More info about a broader issue: istio/istio#16381

Great; I was not aware of this being a broader issue. It sounds like, making the approach canonical and extensible, so that it can be leveraged in other capacities, is a key requirements.

@jasonwzm
Copy link
Member

  1. If a change to common files, propagate new files to repo consumers (e.g. istio/istio, istio/test-infra)

One thing to note that, the branch does matter here. istio/test-infra only has one branch, but the jobs it contains will depend on different images.

@clarketm
Copy link
Member

clarketm commented Nov 12, 2019

One thing to note that, the branch does matter here. istio/test-infra only has one branch, but the jobs it contains will depend on different images.

Good point! To ensure it is clear, for example if a change to files in istio/common-file@release-1.4, then only propagate update (and PR) to istio/<repo>@release-1.4 branch(es). And only update build-tools image for that specific repo and branch jobs?

@jasonwzm
Copy link
Member

One thing to note that, the branch does matter here. istio/test-infra only has one branch, but the jobs it contains will depend on different images.

Good point! To ensure it is clear, for example if a change to files in istio/common-file@release-1.4, then only propagate update (and PR) to istio/<repo>@release-1.4 branch(es). And only update build-tools image for that specific repo and branch jobs?

Yes, that is correct.

@sdake
Copy link
Member

sdake commented Dec 12, 2019

Hi Gang,

I took the first stab at semi-automating this work here:

https://gist.github.com/sdake/2e45b74412bb66e1cff7839f0d80f8d0 free to make improvements.

There is an RFC covering this topic in more far more detail here presented to T&R: https://docs.google.com/document/d/1d4tB3LCeoRcW_TEj_4TMpJSaeSF8H0iSUa5Ln2k1l4I/edit?ts=5deef575#heading=h.xw1gqgyqs5b

@sdake
Copy link
Member

sdake commented Dec 12, 2019

One thing to note that, the branch does matter here. istio/test-infra only has one branch, but the jobs it contains will depend on different images.

Good point! To ensure it is clear, for example if a change to files in istio/common-file@release-1.4, then only propagate update (and PR) to istio/<repo>@release-1.4 branch(es). And only update build-tools image for that specific repo and branch jobs?

Yes, that is correct.

Yup this is what we are after. Personally I think it would be better to walk prior to teleporting to mars, but I leave it to you :)

@clarketm
Copy link
Member

clarketm commented Dec 12, 2019

@sdake

Yup this is what we are after. Personally I think it would be better to walk prior to teleporting to mars, but I leave it to you :)

Thank you for getting the ball rolling on this issue with the RFC and script. I am working on a generic framework to abstract some of the complexity of multi-repo dependency updates. Ideally, we can trigger these routines in CI (on merge/postsubmit) in the future, but I agree that encapsulating this logic in a script is the appropriate first step. (#2172)

@howardjohn
Copy link
Member

We may want to consider not requiring an approved on PR from the bot (provided tests pass of course). otherwise someone will have to go approved 10 prs every time there is a common files change

@clarketm
Copy link
Member

clarketm commented Dec 17, 2019

We may want to consider not requiring an approved on PR from the bot (provided tests pass of course). otherwise someone will have to go approved 10 prs every time there is a common files change

Yes, we can either not require approval or force push (if no conflict). Let's give the solution in #2223 a try at least once first to gauge the toil then we can optimize.

@howardjohn
Copy link
Member

Yes definitely let it bake for a bit. Direct push would bypass tests though so probably don't want that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants