-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Rebase 1.7.6 #16615
Rebase 1.7.6 #16615
Conversation
The only problem I have right now is:
|
That's just because your server isn't far enough to fail on something else :) I think we had a patch somewhere renaming those. |
/unassign |
ad10827
to
74c00ed
Compare
@deads2k found the problem, this was introduced in this fix kubernetes/kubernetes@5b7783f after I removed it it's working. We'll need to figure out if the fix is wrong or we messed something up. I haven't checked how this works in 1.8, though. Let's see how tests results are now. |
@liggitt remember when you've mentioned k8s 1.7 bumped azure driver, now I see this is biting us really hard, docker distribution's azure driver relies on some methods not present anymore in the newer version :/ @smarterclayton fyi. What should we do with it? |
This is why we have to get our dockerregistry built separately, so we can track kube levels of docker, etc. Diamond dependencies are going to become impossible |
Yeah, that's the long term goal. But thankfully I realized we have a vendor directory inside our docker/distribution, which just needs the old Azure dependency and it looks like it's working... |
8dc237c
to
9868522
Compare
This is almost green, except for |
So apparently rebasing the rebase of the master helped. This is ready for reviews. |
@@ -63,6 +63,10 @@ func GetOperationIDAndTags(r *restful.Route) (string, []string, error) { | |||
op := r.Operation | |||
path := r.Path | |||
var tags []string | |||
// TODO: This is hacky, figure out where this name conflict is created and fix it at the root. | |||
if strings.HasPrefix(path, "/apis/extensions/v1beta1/namespaces/{namespace}/") && strings.HasSuffix(op, "ScaleScale") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create issue pls, this is ultraugly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #16635.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mehdy?
Origin changes looks reasonable to me (except #16615 (comment) maybe ;-). Good job! |
@@ -589,15 +589,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag | |||
|
|||
routes := []*restful.RouteBuilder{} | |||
|
|||
// If there is a subresource, kind should be the parent's kind. | |||
if hasSubresource { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should do this. This is messing with the kinds reported via discovery, right?
|
Nope, @liggitt warned me that 1.7.x bumps Azure deps, which in-turn conflict with docker distrubution's requirements. I had to move the old Azure deps to distribution's vendor dir. It's intentional. |
/retest |
1 similar comment
/retest |
LOL, and I thought that I'm the only one hitting:
apparently our CI suffered that one as well. |
/retest |
Failed on:
|
/retest |
/retest |
|
/retest |
1 similar comment
/retest |
@deads2k add a retest-not-required label if you want it merged as quick as possible? |
That carries a risk of breakage, but I can live with that. Do we have an easily accessible view of the branch jobs to see what happens after this merges? |
https://deck-ci.svc.ci.openshift.org/?type=postsubmit These are jobs that run on every commit in master. |
thanks. |
You likely want origin only: https://deck-ci.svc.ci.openshift.org/?repo=openshift%2Forigin&type=postsubmit Btw, I verified that this PR is not conflicting with the current batch in the queue. So as soon as that job finishes, we should see this merged. |
@soltysh failed again /retest |
Automatic merge from submit-queue. |
@soltysh @ironcladlou I need SHAs to start the apimachinery, client-go, apiserver, and metrics release-1.7.6 branches from |
Godeps references commit "a08f5eeb6246134f4ae5443c0593d72fd057ea7c" which doesn't exist under a branch in openshift/kubernetes, which means git fetch can't grab it. Can someone tag this to whatever branch it should be tagged to? |
Another hack/restore-godeps failure:
Godeps/Godeps.json has docker/distribution in there twice at two different Revs. |
Automatic merge from submit-queue. UPSTREAM: google/cadvisor: 1766: adaptive longOp for du operation google/cadvisor#1766 xref https://bugzilla.redhat.com/show_bug.cgi?id=1498632 @derekwaynecarr @ravisantoshgudimetla hold pending #16615
So this is weird.... The referenced commit merged to release-1.7.6 from my original rebase PR in openshift/kubernetes@a08f5ee: "soltysh merged 166 commits into openshift:release-1.7.6 from ironcladlou:rebase-1.7.6" Notice that openshift/kubernetes@a08f5ee isn't in the release-1.7.6 branch now. BUT, here's a duplicate of the commit with a different sha in the release-1.7.6 branch openshift/kubernetes@ad0dbed. 👀 |
@deads2k and I were syncing our forks, either of us must have overwritten the commits. Looking at the commits it appears it was me, not sure why though, since I was working on top of freshly pulled branch... |
@deads2k @liggitt @ironcladlou