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

Rebase kubernetes 1.8.1 #43

Merged
merged 68 commits into from
Nov 20, 2017
Merged

Conversation

mfojtik
Copy link

@mfojtik mfojtik commented Oct 12, 2017

Rebase for kube 1.8.1

@mfojtik
Copy link
Author

mfojtik commented Oct 12, 2017

@deads2k @liggitt @sttts @soltysh PTAL (feel free to review as I go, I hope to have 100% of the list done today).

@@ -142,6 +142,30 @@ var ResourcesShortcutStatic = []ResourceShortcuts{
ShortForm: schema.GroupResource{Group: "extensions", Resource: "psp"},
LongForm: schema.GroupResource{Group: "extensions", Resource: "podSecurityPolicies"},
},
{
ShortForm: schema.GroupResource{Group: "apps.openshift.io", Resource: "dc"},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with shortcuts in discovery, do we still need this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with shortcuts in discovery, do we still need this?

@mfojtik we support one level of skew. I think we supported shortnames in kube 1.7, we just need to make sure we've wired them.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfojtik we support one level of skew. I think we supported shortnames in kube 1.7, we just need to make sure we've wired them.

Here openshift/origin#16823

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! i will drop this carry

@sttts
Copy link

sttts commented Oct 12, 2017

How do UPSTREAM: <carry>: update clientset generator for openshift groups and UPSTREAM: <carry>: update client namer rules for amguity relate? Also UPSTREAM: <carry>: update group references.

@deads2k
Copy link

deads2k commented Oct 12, 2017

@@ -110,12 +110,9 @@ kubernetes.tar.gz
# generated files in any directory
# TODO(thockin): uncomment this when we stop committing the generated files.
#zz_generated.*
zz_generated.openapi.go
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the actual openapi file missing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sttts @soltysh said I should add it in a commit to the end of this rebase.

@mfojtik
Copy link
Author

mfojtik commented Oct 12, 2017

@deads2k sure, I'm tracking this locally in text doc, but will upload that to spreadsheet

@mfojtik mfojtik force-pushed the rebase-1.8.1 branch 2 times, most recently from 52fbeef to 123cc2a Compare October 12, 2017 14:31
@liggitt
Copy link

liggitt commented Oct 12, 2017

don't forget the checklist in openshift/origin#16065

@liggitt
Copy link

liggitt commented Oct 12, 2017

we can drop UPSTREAM: <carry>: Tolerate node ExternalID changes with no cloud provider, we've unified with upstream for nodes being able to delete/create themselves

@mfojtik
Copy link
Author

mfojtik commented Oct 12, 2017

I plan to squash all codegen/client-gen carries into one commit. Also need to sort out the 78caf9d31d UPSTREAM: <carry>: allow controller context injection to share informers which seems to drift significantly in upstream...

@liggitt
Copy link

liggitt commented Oct 12, 2017

we can drop UPSTREAM: 45294: Fix protobuf generator for aliases to repeated types

@liggitt
Copy link

liggitt commented Oct 12, 2017

UPSTREAM: 49137: proxier is it really nil merged looking different... can we drop it?

@liggitt
Copy link

liggitt commented Oct 12, 2017

this merged, can we drop UPSTREAM: 48960: No warning event for DNSSearchForming?

@mfojtik
Copy link
Author

mfojtik commented Oct 12, 2017

@liggitt dropped and updated the spreadsheet, thanks!

@liggitt
Copy link

liggitt commented Oct 12, 2017

squash

UPSTREAM: <carry>: update namespace lifecycle to allow review APIs
UPSTREAM: <carry>: ignored namespace lifecycle resources

@mfojtik mfojtik force-pushed the rebase-1.8.1 branch 2 times, most recently from bf7902d to 93448eb Compare October 12, 2017 15:51
@mfojtik
Copy link
Author

mfojtik commented Oct 12, 2017

@liggitt squashed and also squashed the client-gen/codegen carries into one.

@mfojtik
Copy link
Author

mfojtik commented Oct 16, 2017

tracking integration failures (note this is result of make test-integration, I will try to re-run the failed ones)

TestCustomResourceCascadingDeletion
E1016 09:21:51.819755   14346 garbagecollector.go:259] error syncing item &garbagecollector.node{identity:garbagecollector.objectReference{OwnerReference:v1.OwnerReference{APIVersion:"mygroup.example.com/v1beta1", Kind:"foo97227a", Name:"dependentcc967", UID:"6ebe74e9-b253-11e7-82f3-0ebee34bef42", Controller:(*bool)(nil), BlockOwnerDeletion:(*bool)(nil)}, Namespace:"crd-cascading"}, dependentsLock:sync.RWMutex{w:sync.Mutex{state:0, sema:0x0}, writerSem:0x0, readerSem:0x0, readerCount:1, readerWait:0}, dependents:map[*garbagecollector.node]struct {}{}, deletingDependents:false, deletingDependentsLock:sync.RWMutex{w:sync.Mutex{state:0, sema:0x0}, writerSem:0x0, readerSem:0x0, readerCount:0, readerWait:0}, beingDeleted:false, beingDeletedLock:sync.RWMutex{w:sync.Mutex{state:0, sema:0x0}, writerSem:0x0, readerSem:0x0, readerCount:0, readerWait:0}, owners:[]v1.OwnerReference{v1.OwnerReference{APIVersion:"mygroup.example.com/v1beta1", Kind:"foo97227a", Name:"ownern2xkc", UID:"6ebe3db0-b253-11e7-82f3-0ebee34bef42", Controller:(*bool)(nil), BlockOwnerDeletion:(*bool)(nil)}}}: rpc error: code = Internal desc = transport: write unix @->localhost:33317419759551005710: write: broken pipe

EDIT: This passed when ran just this test manually. No other integration failures.

@soltysh
Copy link

soltysh commented Oct 16, 2017

UPSTREAM: <drop>: merge multiple registrations for the same group we are going to carry this patch as long as we have our legacy API, which is quite a few versions. Maybe we should just rename it to carry in that case?

@soltysh
Copy link

soltysh commented Oct 16, 2017

@deads2k these:

  • UPSTREAM: 00000: make AsVersionedObjects default cleanly - I can't find any changes like that in upstream where they ever proposed/rejected? I'd like to describe this commit accordingly as carry or an actual upstream PR
  • UPSTREAM: 00000: disambiguate operation names for legacy discovery this looks like a carry patch, is it?

@deads2k
Copy link

deads2k commented Oct 16, 2017

@deads2k these:

UPSTREAM: 00000: make AsVersionedObjects default cleanly - I can't find any changes like that in upstream where they ever proposed/rejected? I'd like to describe this commit accordingly as carry or an actual upstream PR
UPSTREAM: 00000: disambiguate operation names for legacy discovery this looks like a carry patch, is it?

Both have a shot at being accepted upstream, though I think AsVersionedObjects has an open pull refactoring it.

@soltysh
Copy link

soltysh commented Oct 16, 2017

So I'm missing following 3 commits here:

  1. UPSTREAM: <carry>: Fix to avoid REST API calls at log level 2.
  2. UPSTREAM: <carry>: Do not error out on pods in kube-system
  3. UPSTREAM: <carry>: allow controller context injection to share informers

All the rest lgtm.

@liggitt
Copy link

liggitt commented Oct 16, 2017

I think AsVersionedObjects has an open pull refactoring it.

kubernetes#53861 moves and unexports it

@mfojtik
Copy link
Author

mfojtik commented Oct 16, 2017

@soltysh

  1. 4e9886f
  2. still investigating
  3. 3c51e41

deads2k and others added 17 commits November 7, 2017 14:17
:100644 100644 b3995e59dd... 7ed48812b9... M	plugin/pkg/admission/webhook/admission.go
…to work without modifying api

:100644 100644 d6e45cb292... a6e862366b... M	pkg/apis/admissionregistration/validation/validation.go
:100644 100644 7ed48812b9... 8281825649... M	plugin/pkg/admission/webhook/admission.go
:100644 100644 8dee1f3a05... c423b67842... M	plugin/pkg/admission/webhook/admission_test.go
:100644 100644 baa1e2a045... 748c7009f8... M	test/e2e/framework/framework.go
:100644 100644 fed687e9bd... bcc87f4fe0... M	staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go
:100644 100644 28a8d1f652... 895fd233a9... M	pkg/kubelet/kubelet_pods.go
:100644 100644 ed9301afe0... f2809348bf... M	pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go
:100644 100644 063a89d6b3... 28a831d848... M	pkg/kubelet/kuberuntime/kuberuntime_gc.go
:100644 100644 389b1e52cc... 4a419524dd... M	pkg/kubelet/kuberuntime/kuberuntime_gc_test.go
:100644 100644 7e5d1d0d29... 7286bf44c0... M	pkg/kubelet/kuberuntime/kuberuntime_manager.go
:100644 100644 eec16ea18a... bba5bc4ced... M	pkg/printers/customcolumn.go
:100644 100644 dc1e72101c... 94f915c0d9... M	pkg/printers/customcolumn_test.go
:100644 100644 895fd233a9... 162030fe5b... M	pkg/kubelet/kubelet_pods.go
…mapper

:100644 100644 c5980bd... f9853d6... M	pkg/kubectl/cmd/util/factory_test.go
:100644 100644 2a7c7bc... d3d8b50... M	pkg/kubectl/cmd/util/shortcut_restmapper.go
:100644 100644 20d671e... 3f5bcce... M	pkg/kubectl/cmd/util/shortcut_restmapper_test.go
:100644 100644 9d7a8fc... 49bbc93... M	pkg/kubectl/kubectl.go
… tolerations by namespace level empty (default, whitelist) tolerations.
…rrect symlinks

Symlinks relative to a working directory were being constructed to the
wrong location, leading to failure to refresh client certs.
The Get() function of non-namespace lister passes a temporary object to
indexer.Get() in order to fetch the actual object from the indexer. This
may cause Go to allocate the temporary object on the heap instead of the
stack, as it is passed into interfaces. For non-namespaced objects,
Get(&Type{ObjectMeta: v1.ObjectMeta{Name: name}}) should be equivalent
to GetByKey(name).

This could be the root cause of excessive allocations, e.g. in tests
clusterRoleLister.Get() has trigger 4 billion allocations. See
openshift/origin#16954

Signed-off-by: Christian Heimes <[email protected]>
…journald log driver

When using the legacy docker container runtime and when a container has
terminationMessagePolicy=FallbackToLogsOnError and when docker is
configured with a log driver other than json-log (such as journald),
the kubelet should not try to get the container's log from the
json log file (since it's not there) but should instead ask docker for
the logs.
@mfojtik
Copy link
Author

mfojtik commented Nov 7, 2017

@gnufied without that pick the tests are passing, do we still need it? if yes it seems that it needs some rework for 1.8.x

EDIT: solved by picking the missing commits.

gnufied and others added 6 commits November 7, 2017 15:07
We should be careful while generating multiattach errors.
We seem to be generating too many of them because old code
had minor bug.
:100644 100644 492ca3d6b2... 2ee9a4bbb1... M	pkg/controller/volume/attachdetach/reconciler/reconciler_test.go
:100644 100644 48b5d52550... 7f1327d3da... M	pkg/kubectl/cmd/testing/fake.go
:100644 100644 05014de60d... 29c63bcec5... M	pkg/kubectl/cmd/util/factory.go
:100644 100644 abb10d52bb... 01ad1ff2a3... M	pkg/kubectl/cmd/util/factory_object_mapping.go
@mfojtik mfojtik mentioned this pull request Nov 16, 2017
19 tasks
@mfojtik mfojtik force-pushed the rebase-1.8.1 branch 2 times, most recently from 934687d to 0d5291c Compare November 20, 2017 14:40
@liggitt
Copy link

liggitt commented Nov 20, 2017

LGTM

@liggitt liggitt merged this pull request into openshift:release-1.8.1 Nov 20, 2017
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.