Update vendored k8s.io packages to target release-1.8/release-5.0 branches #15
Conversation
* use recommendConfig * following https://github.com/kubernetes/sample-apiserver/tree/release-1.8
jetstack-bot
added
the
release-note
jetstack-bot
added
the
size/XXL
|
@munnerz I am not too sure how to implement the SkipComplete with this updated code base. Maybe you can give me a hint or at least tell the trade off, why we had it before |
munnerz
left a comment
munnerz
left a comment
There was a problem hiding this comment.
Just the one comment about ListMeta's optional tag. If you update that and ping me I'll add a lgtm label @simonswine
| metav1.TypeMeta | ||
| metav1.ObjectMeta | ||
| // +optional | ||
| metav1.ListMeta |
There was a problem hiding this comment.
Don't think this is, or should be, optional?
There was a problem hiding this comment.
Just copied from the core:
https://github.com/kubernetes/kubernetes/blob/master/pkg/api/types.go#L3409
| // New returns a new instance of WingServer from the given config. | ||
| func (c completedConfig) New() (*WingServer, error) { | ||
| genericServer, err := c.Config.GenericConfig.SkipComplete().New("wing", genericapiserver.EmptyDelegate) // completion is done in Complete, no need for a second time | ||
| genericServer, err := c.GenericConfig.New("wing", genericapiserver.EmptyDelegate) |
There was a problem hiding this comment.
Not using 'SkipComplete' looks good to me, as we already store a reference to the completed config on L68 (40390ed#diff-eeee2947c8ccb06990948a4196759b69R68).
In the previous code we threw away the completedConfig structure returned from cfg.GenericConfig.Complete, which meant we had to re-construct this completed configuration (albeit this time without calling Complete - I assume this was okay because Complete was originally called on a pointer? Not sure as I've not checked the code for Complete yet.
There was a problem hiding this comment.
Just wanted someone to double check
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: munnerz, simonswine 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 |
jetstack-bot
added
the
approved
|
Automatic merge from submit-queue. |
|
Removing label |
|
Removing label |
…rver-port Disable API server's insecure port binding
Fix naming of apply fragments
4 participants
What this PR does / why we need it:
Pins k8s.io dependencies to release-1.8 branches which are hopefully more stable, better tested and better integrated than picking a random commit from the master branch of each project.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Fixes: https://gitlab.jetstack.net/tarmak/tarmak/issues/138
Replaces: #14
Special notes for your reviewer:
Compared to #14 this reuses the way how we build the API client and listers/informers. Wing server runs in standalone only mode, we never use delegated auth
Release note: