search

openshift / origin

Conformance test suite for OpenShift
http://www.openshift.org
Apache License 2.0
8.5k stars 4.7k forks source link

Fork branches #16291

Closed deads2k closed 6 years ago

deads2k commented 7 years ago

We should have forks for the kubernetes repos that we carry patches for (k8s.io/kubernetes, k8s.io/apimachinery, k8s.io/client-go, etc). https://github.com/deads2k/sync-vendor/blob/master/hack/sync-vendor.sh provides an example of how we can move the commits fairly efficiently, but it highlighted a problem pulling them directly from openshift/origin. You lose common ancestors with the upstream.

Because we apply patches on top of a particular kube tag, we can't have a single 1.7.x branch, but we could create a branch for every level we pull. For instance, we could have an openshift/kubernetes:release-1.7.0 and an openshift/kubernetes:release-1.7.5. Doing that, using sync-vendor to keep it up to date would allow us to pick into the openshift/kubernetes-apimachinery:release-1.7.0 and have a common ancestor. Our master branches (tracking openshift/origin:master maybe?) would end up re-writing history as we start pulling different levels of kube, but having the common ancestor with the upstream would be really nice.

@ironcladlou @smarterclayton @liggitt @sttts @mfojtik @tnozicka arguments for or against?

mfojtik commented 7 years ago

CC @tnozicka

stevekuznetsov commented 7 years ago

git submodule solves this problem in the other direction. Can we rethink the issue in that way to save the engineering cost by 100% on these tools?

deads2k commented 7 years ago

Bumping to p1. This decision gates how we create the forks to support a client-go which is required to split anything out of tree and the pieces around it are ready now.

enj commented 7 years ago

@simo5 you probably have some opinions on this.

simo5 commented 7 years ago

I am for git submodules, importing whole chunks of code in different repos (we have more than one now) becomes quickly insane as each repo then has to figure out if any fixes are necessary and what not. We should, instead have an official kube fork in an openshift tree and have all other openshift trees refer to it by a submodule. This way when the oficial fork we rely on is updated all we need to do is run a bot that opens PRS to update the submodule reference, and all projects update basically for free to the same bits. Bonus points this makes it easy to: a) prevent just one of the trees importing a patch to the "common" tree without the others syncing up and creating support issues by having different project have different behavior b) Have CI test in all supported projects when the bot bumps the submodule value, this way guaranteeing an update to the common kube tree does not break one of the projects long down the road, but flags it immediately as an issue on the "common" tree and the project

liggitt commented 7 years ago

git submodule solves this problem in the other direction. Can we rethink the issue in that way to save the engineering cost by 100% on these tools?

Submodules are one way we could consume the split components. This issue is not about how we consume those split repos, but how we produce them. They do not exist today.

We should, instead have an official kube fork in an openshift tree and have all other openshift trees refer to it by a submodule. This way when the oficial fork we rely on is updated all we need to do is run a bot that opens PRS to update the submodule reference, and all projects update basically for free to the same bits.

For better or worse, Kube develops in a monorepo, as do we. What you describe would mean picking one change from upstream would require N PRs to openshift kube forks instead of one to the origin repo. It also would mean that on rebases, related changesets split across those repos would no longer be coherently testable/inspectable.

simo5 commented 7 years ago

@liggitt if you have multiple trees you have multiple PRs no matter what. With submodules:

With vendoring:

The advantage I see with submodules:

With vendoring the risk is trees getting out of sync as one PR passes and the others don't and changes are made to one tree but not another

For example can you tell me if the oauth-proxy project has the same vendored bits as the origin project without some deep inspection ?

From the POVs of testing it sounds like it wouldn't change much, any churn that may derive from CI failures in projects related to kube common tree issues is confined to the common tree and is immediately retested in all other trees as the submodules bot files automatic PRs for submodule reference update.

liggitt commented 7 years ago

Human driven actions (picks/rebases) should deal with one repo (edit: as long as our CI systems are unable to deal with multi-repo test, merge, and dependency ref bumps). Bot-driven actions (splitting/publishing/revendoring) can deal with some multirepo issues, but not all

simo5 commented 7 years ago

@liggitt The way I read it looks like you are agreeing with submodules, but I think you aren't ?

enj commented 7 years ago

I believe this is related:

Must we "reverse vendor" the same way Kube does? By reverse vendor I mean having symbolic links inside the vendor directly that point to staging folders outside of vendor. AFAIK the folders in the staging directly are the actual source of truth for those "repos" and the actual repos that are stored in git are kept in sync via bots. So changes to those vendor/staging "repos" inside of Kube causes the external changes to the other git repos. Thus it is exactly opposite of the normal flow of vendored dependencies. I assume this is because those repos do not have stable public APIs / Kube is a monorepo.

liggitt commented 7 years ago

If all we did was vendor release branches of the split repos, and carried no upstream commits, that could work. That is not what we do. Picking and maintaining those upstream commits and trying to stitch back together coherent changesets from split repos upstream is not feasible

simo5 commented 7 years ago

Sorry I do not understand how applying a backport to submodule openshift/kubernetes rather than origin/vendor/kubernetes makes any substantial difference.

Just to make it clear, I am not advocating to submodule upstream kubernetes, but our own fork of it, which we will control and rebase as necessary.

deads2k commented 7 years ago

@simo5 are you suggesting patching openshift/kubernetes instead of starting with openshift/origin?

A process which does not start with, "openshift/origin decides that an upstream patch is ok and merges it into openshift/orign first" won't work well. Our experience revendoring levels of kubernetes and based on picking upstream pull requests is that many important changes require corresponding updates to openshift. If we were to allow people to patch the upstream fork first and only pass CI there, we'd end up with a level of kubernetes which could not be vendored and built into openshift. To make it worse, the changes could become incompatible and people trying to do their picks would get stuck on each other.

To solve this, we can merge into openshift first and synchronize out a level that we know works properly so that we never end up in a broken state.

simo5 commented 7 years ago

Well the way we work now is to have patches against upstream kubernetes in our PR and to backport them into the vendored tree after they have been accepted upstream, often as part of a Pr with other patches.

We can do the same with a submodule, although we would have a PR against openshift/kubernetes and a PR in openshift/origin that depends on the othe PR.

In my mind all we need to do is to make it easy to deal with this dependency, the rest doesn't really change much. To do that what we can do is to allow a PR to have a submodule reference changed to a PR branch while the patch is being worked on but have the gating mechanism of course prevent that pathcset to be merged until that reference is changed to opne that points to the main openshift/kubernetes tree HEAD.

It may be slightly more annoying process in some cases as you have to deal with 2 PRs if you are updating bits on both sides, but I do not see it as a big issue, YMMV though.

Edit: replying to @deads2k

deads2k commented 7 years ago

We can do the same with a submodule, although we would have a PR against openshift/kubernetes and a PR in openshift/origin that depends on the othe PR.

How would you force these two things to be linked today? We don't currently have CI that can do this. You couldn't even create the proper commit for the vendor bump until the first merged, so I'm not sure how you'd accurately create the origin pull.

It may be slightly more annoying process in some cases as you have to deal with 2 PRs if you are updating bits on both sides, but I do not see it as a big issue, YMMV though.

Not having both pieces merge together presents a high risk of not having working levels. As the guy who has put humpty dumpty back together in the past, I think its a critical issue.

simo5 commented 7 years ago

Well the nice thing of submodules is that the submodule reference is updated via a PR, so if you update openshift/kubernetes and the resulting code does not work with openshift then the "automatic PR" will not be merged as tests will fail, and the submodule update can only be merged once (and probably within) the PR that brings origin back in conformance.

So the kubernetes PR part is independent and carries no immediate risk (as orign/master will keep referencing the older kubes commit until tests pass).

The worst case scenario is somethign bad is meged in the kube tree that you ant to back out because it doesn't work at all, and that worst case is handle with a revert PR in openchift/kubernetes.

liggitt commented 7 years ago

The issue is that one pick from kube would result in N PRs to the various fork repos which all have to be tested and merge in unison, then be vendored back in in unison.

The worst case scenario is somethign bad is meged in the kube tree that you ant to back out because it doesn't work at all, and that worst case is handle with a revert PR in openchift/kubernetes.

The worst case is that the split PRs get interleaved or partially merged and we end up with incoherent levels that require manual verification and untangling. We should not operate that way before upstream does.

stevekuznetsov commented 7 years ago

Yeah my original comment about submodules was that they solve the problem in the other direction, so if we could frame it that way we would end up with a lot smaller engineering burden. As it stands we have a lot of weird git untangling to do in the first place to do the reverse vendor breakout. David convinced me that it will be less painful than trying to go the other way, though, with pretty much the same argument as Jordan just made.

stevekuznetsov commented 7 years ago

I guess the other approach would be to push upstream to stop reverse vendoring and make the burden smaller by removing the problem. Do we have a clear roadmap for that? As I understand it (which is poorly) some of the import verification tools were built to ensure that the code in these repos stays separate. If those are in place, are we just waiting for enough stability that sweeping changes do not need to be made to N component repos at once (which can happen today by changing the staging area in one PR) ?

simo5 commented 7 years ago

I guess I do not understand the problem well enough to make a case, so I'll accept whatever you guys think is best. But I think the current situation is far from ideal and causes burden on people tasked to handle other trees like oauth-proxy that also have to vendor k8s.io stuff (clients) and now have to do it on their own. We have to hope to catch when origin moves and stay coherent, with all the problems of dealing with breakage, but w/o the luxury of having the person responsible for the breakage handling it at the time the change is made.

sttts commented 7 years ago

I guess the other approach would be to push upstream to stop reverse vendoring and make the burden smaller by removing the problem. Do we have a clear roadmap for that?

The roadmap is that there seems to be a consensus in sig-arch to start with removing the staging/ approach with 1.10 (January time frame). But, as far as I know there is no agreement and active work towards a workflow with CI using multiple repos. So I don't see this happening soon'ish (January is 3 months away!). About stable APIs for such a develoment workflow: every release until now had serious code-incompatible changes between repos. While it's getting better, we have to become much more conservative about fixing APIs in such a way. Compare our client-go semver versioning: v2.0.0 (1.5), v3.0.0 (1.6), v4.0.0 (1.7), v5.0.0 (1.8).

sttts commented 7 years ago

Catching up with the discussion here, we mix two completely unrelated things:

  1. submodules vs. patches against vendor/k8s.io/kubernetes
  2. forks of the kube staging/ repos (@deads2k's original question in this issue).

About (1) under the assumption that we update our forks (2):

Submodules would help to enforce that openshift/kubernetes is up-to-date and people can vendor it at any time. The PR process would probably be like this (assuming we continue to use only one submodule and vendor/k8s.io/* symlinks for the staging repos):

The alternative is to keep our UPSTREAM commits in origin and export those to openshift/kubernetes

In either case, we have to export the staging/ repos of openshift/kubernetes to our forks. A bot/script doing this will probably always just do fast-foward cherry-picks of new commits to a known export branch (possibly named like the origin branch they come from or like @deads2k proposes with branch names based on the upstream tag the branch is based on). For this we could put a branch mapping file into origin, e.g. vendor/k8s.io/kubernetes-branch. The bot/script would pick that up and do the cherry-picking.

When we switch the upstream tag on rebase, we have to

Preparation of the branch(es) means to rebase our UPSTREAM patches onto the new upstream base commit. While we can provide a script to do that for minor releases of kube, in general I don't expect this to be automatic as UPSTREAM patch porting will still be an issue. Also the decision to drop certain commits will not go away. Probably we can start fresh with the upstream tag for (C) and let the bot do the cherry-picking onto the staging/ repos.

Also note even with a continuously running bot, we don't get easy vendor'ability of origin for free: we would still have UPSTREAM commits, and it is not obvious how we can easily make the bot or CI to update origin to point to the new forks continuously. We could make the bot update origin's Godeps/Godeps.json, but leave the UPSTREAM commits in place. Or we could continuously let it revendor kube with a new sync(k8s.io/kubernetes) commit. The later would be a similar two-way sync as in (iv b) and (iv c) above.

simo5 commented 7 years ago

@sttts excellent explanation, this is more or less what I was hinting at indeed. Thanks also for making it clear we diverged and started talking about 2 things at the same time.

openshift-bot commented 6 years ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

soltysh commented 6 years ago

This is already happening and is synced fairly frequently. /close