Skip to content

WIP: Sprint 60 rebase#1428

Closed
pweil- wants to merge 31 commits intoopenshift:masterfrom
pweil-:rebase
Closed

WIP: Sprint 60 rebase#1428
pweil- wants to merge 31 commits intoopenshift:masterfrom
pweil-:rebase

Conversation

@pweil-
Copy link

@pweil- pweil- commented Mar 24, 2015

WIP for the rebase. Tagging for reviewers

@smarterclayton @pmorie @abhgupta @fabianofranz

The WTF did you do Paul? tracker: https://gist.github.com/pweil-/0753309e70f0604dd758 (rebase actions only, this gist doesn't contain any refactoring info)

@pweil- pweil- force-pushed the rebase branch 4 times, most recently from 42b25d2 to 688c92a Compare March 25, 2015 19:51
@derekwaynecarr
Copy link
Member

Can we pull over NamespaceLifecycle admission control plugin as part of this change?

@pweil-
Copy link
Author

pweil- commented Mar 26, 2015

I pulled a mulligan this morning so I should have everything as of a couple hours ago in my next commit. Will that cover it?

@derekwaynecarr
Copy link
Member

If you can add an anonymous import to the new plugin here:

https://github.com/openshift/origin/blob/master/pkg/cmd/server/start/start_allinone.go

It should cause it to be pulled over.

@pweil-
Copy link
Author

pweil- commented Mar 26, 2015

ok, I'll do that when I get the unit tests stable.

@derekwaynecarr
Copy link
Member

Thanks!

@pweil- pweil- force-pushed the rebase branch 2 times, most recently from f4f3576 to 12c8b9f Compare March 26, 2015 19:58
@pweil-
Copy link
Author

pweil- commented Mar 26, 2015

@smarterclayton @pmorie @abhgupta @fabianofranz - I think I'm getting close (I hope). I had a successful run of test-go and test-cmd with what is currently in this pr and will be checking out test-end-to-end next

@derekwaynecarr I noticed a weird issue with deleting a project in test-cmd. It went into the terminating state as expected but hung there for at least a minute. Issuing a second delete killed it immediately so my last commit has 2 deletes in test-cmd.sh which is wrong (but working). Any advice there?

@derekwaynecarr
Copy link
Member

@pwell - it will hang for a minute until the namespace controller detects that its terminating and cleans up the content in the project. I can't really improve this to make it faster until we get this rebased because origin needs to also take part in the clean-up life-cycle.

@pweil- pweil- force-pushed the rebase branch 2 times, most recently from a000a81 to a887668 Compare March 27, 2015 13:09
@pweil-
Copy link
Author

pweil- commented Mar 27, 2015

@smarterclayton @pmorie @abhgupta @fabianofranz PTAL - light is green, think this is ready for a review. 2nd to last commit was a merge from master to pick up today's PRs. If that was incorrect please let me know the proper procedure (hopefully not a start over but if so that's not terrible now that I've started over about 10 times)

@smarterclayton
Copy link
Contributor

That's all the comments I had

@smarterclayton
Copy link
Contributor

[test]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1533/)

@fabianofranz
Copy link
Member

LGTM after squashing.

@derekwaynecarr
Copy link
Member

kicked jenkins

@smarterclayton
Copy link
Contributor

W0330 19:05:44.064095   25692 status_manager.go:59] Failed to updated pod status: error updating status for pod "deploy-php-10cx0o": Pod "deploy-php-10cx0o" is invalid: status.host: invalid value '': pod host cannot be changed directly

Something seriously wrong there.

@openshift-bot
Copy link
Contributor

Evaluated for origin up to a24b130

@pweil-
Copy link
Author

pweil- commented Mar 31, 2015

Light is green, issue was fixed upstream so I've committed it with an UPSTREAM tag. There are still quite a few of the Failed to updated pod status errors though. Not sure what is causing those yet.

@derekwaynecarr
Copy link
Member

I thought host was immutable on pod. Will look to see if that is case in Kube

@derekwaynecarr
Copy link
Member

https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/api/validation/validation.go#L772

So somewhere we are sending a pod with a host that is set and then we are attempting to update some other part of status ( maybe a label? ) and we are not passing the set host.

@pweil-
Copy link
Author

pweil- commented Mar 31, 2015

It should be (for now, there is a TODO above the validation). I do not get these errors on test-cmd.sh or test-e2e locally for some reason and Jenkins isn't seeing it either.

@smarterclayton
Copy link
Contributor

I'm going to cut a release and then prepare to merge. Do any cleanup you need to this PR

----- Original Message -----

It should be (for now, there is a TODO above the validation). I do not get
these errors on test-cmd.sh or test-e2e locally for some reason and Jenkins
isn't seeing it either.


Reply to this email directly or view it on GitHub:
#1428 (comment)

@derekwaynecarr
Copy link
Member

We think the status_manager error is due to lack of this commit:

kubernetes/kubernetes@372c5c9#diff-bf28da68f62a8df6e99e447c4351122d

It only affects reporting status while a pod is in pending state and docker container info cant be queried.

@pmorie
Copy link
Contributor

pmorie commented Mar 31, 2015

@smarterclayton @derekwaynecarr @pweil- @ironcladlou I just discussed squashing this branch with @pweil- at some length on the phone. I do not think there is a good way to squash these commits without doing a soft reset and breaking the changes after the godep bump and upstream patches into a single commits or commits based on sets of related packages. The downside is that we will lose visibility into the commits pulled in and there will then be a rebase conflict with master (if I understand correctly). If we want to keep this as a PR which is merged normally, it should probably go in as-is (with maybe an additional commit to clean up any things in the state of the branch which need it). The alternative is squashing, which is going to have a ton of merge issues that need to be sorted out one by one.

@pweil-
Copy link
Author

pweil- commented Mar 31, 2015

testing out a cleaned up version here: #1519.

WTF did you do paul:

  1. new branch based on origin/master
  2. cherry-pick bump
  3. resolve conflicts with bump commit
  4. cherry-pick all UPSTREAM commits
  5. merge pweil/rebase
  6. resolve pweil/rebase commit conflicts

@pweil-
Copy link
Author

pweil- commented Mar 31, 2015

closed in favor of #1519

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a24b130 on pweil-:rebase into * on openshift:master*.

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 this pull request may close these issues.

9 participants