-
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.8.1 #17115
Rebase 1.8.1 #17115
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: soltysh Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
api/types/registry \ | ||
api/types/strslice \ | ||
api/types/swarm \ | ||
api/types/versions |
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.
wow, if this goes away finally
@@ -68,6 +65,9 @@ type Serializer struct { | |||
var _ runtime.Serializer = &Serializer{} | |||
var _ recognizer.RecognizingDecoder = &Serializer{} | |||
|
|||
// TODO: Remove jsoniter for now as it is buggy and fails the serialization | |||
// test for DockerImage objects. | |||
/* |
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.
Can you add more info to the commit message? What fails? Do we have a test-case? Can we open an issue in the upstream project?
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.
@mfojtik fyi, since you've touched it
@@ -784,7 +786,7 @@ func GetAuditConfig(auditConfig configapi.AuditConfig) (audit.Backend, auditpoli | |||
// backwards compatible writer to regular log | |||
writer = cmdutil.NewGLogWriterV(0) | |||
} | |||
backend = auditlog.NewBackend(writer, auditlog.FormatLegacy) | |||
backend = auditlog.NewBackend(writer, auditlog.FormatLegacy, auditv1alpha1.SchemeGroupVersion) |
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.
why not the beta version?
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 want to do it in the rebase, I'll do it post, I have a trello card for it.
if err != nil { | ||
return nil, errors.NewInternalError(fmt.Errorf("unable to connect to server: %v", err)) | ||
} | ||
streamOptions := remotecommand.StreamOptions{ | ||
SupportedProtocols: kubeletremotecommand.SupportedStreamingProtocols, |
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.
surprising
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.
That's still broken, that's one of the pieces I need to revisit.
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.
@soltysh is there a follow up issue or note somewhere so we don't forget?
Re-synced k8s bump commit and rebased against this morning master, but it appears there are some problems in the k8s pr. I'll sync with @mfojtik tomorrow and figure this out. |
533ef94
to
419f26d
Compare
pkg/cmd/server/origin/master.go
Outdated
@@ -194,7 +194,7 @@ func (c *MasterConfig) newOAuthServerHandler(genericConfig *apiserver.Config) (h | |||
} | |||
|
|||
func (c *MasterConfig) withAggregator(delegateAPIServer apiserver.DelegationTarget, kubeAPIServerConfig apiserver.RecommendedConfig, apiExtensionsInformers apiextensionsinformers.SharedInformerFactory) (*aggregatorapiserver.APIAggregator, error) { | |||
aggregatorConfig, err := c.createAggregatorConfig(kubeAPIServerConfig) | |||
aggregatorConfig, err := c.createAggregatorConfig(kubeAPIServerConfig.Config, c.ClientGoKubeInformers) |
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.
Why isn't kubeAPIServerConfig.SharedInformerFactory set with a non-nil value?
This change looks suspicious, probably now hiding another error.
d141711
to
036aca0
Compare
…ump cronjob storage
8e9bea0
to
614724c
Compare
CI is green on 8e9bea0 modulo existing issues against master Squashed commits to 614724c (zero diff with 8e9bea0) no conflicts in the two commits between 79097d0...3841e1d |
@soltysh: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
@mfojtik @sttts @liggitt @deads2k @ironcladlou you can slowly start looking into it.
Rebase status:
make build
make build-test
follow ups:
images/dockerregistry
, resolve pulling docker-registry for e2e? - Remove dockerregistry Dockerfile and build #17388source-to-image
to stop using engine-api - Drop engine api source-to-image#834oc cluster
to stop using engine-apidocker.io/...
- opened bzrelease notes - added to openshift/openshift-docs#6458
{}
->[] breaks)