Refactor internal API for Pods to match v1beta3 #2097
Conversation
|
Extracted from #1600 |
markturansky
changed the title
markturansky
changed the title
markturansky
force-pushed
the
v1beta3_podrefactor
branch
from
ad86d24 to
8d5a35a
Compare
|
Please see #2102 and rebase. Metadata was inlined in the internal object representation, in part in order to make this kind of thing somewhat easier. |
|
@bgrant0607 on it. this pull is a work in progress. |
|
@bgrant0607 pull #2086 is part of the same overall v1beta3 refactor, if you wouldn't mind reviewing it for me. |
markturansky
force-pushed
the
v1beta3_podrefactor
branch
3 times, most recently
from
a9a3493 to
811288b
Compare
|
#2086 has been merged. Please rebase this. |
|
I'm going to handle replication controllers and pod templates - so you should be able to just focus on the pod aspect. |
|
Also, I'm going to have to deal with a chunk of pod spec conversion so if you hold up a bit on this refactor you'll get some stuff for free. Maybe work on the other objects beyond pod and repl controller? |
|
Watch #2195 for changes to validation and types added (and some roundtrip cleanup). |
|
After yesterday's PodStatus->PodCondition refactor, I was going to tackle pods today. I have much of it done already. Perhaps it will come together quickly for me this morning and be ready for you shortly. |
markturansky
force-pushed
the
v1beta3_podrefactor
branch
6 times, most recently
from
fb1a781 to
640a57c
Compare
markturansky
force-pushed
the
v1beta3_podrefactor
branch
6 times, most recently
from
f646a09 to
0ff6480
Compare
| } | ||
| pod.DesiredState.Manifest.ID = pod.Name | ||
| pod.UID = pod.Name |
There was a problem hiding this comment.
This is wrong and should be removed. FillObjectMetaSystemFields sets UID, you shouldn't be changing it.
There was a problem hiding this comment.
Line 100 above removed.
markturansky
force-pushed
the
v1beta3_podrefactor
branch
3 times, most recently
from
5904e0e to
c3eb248
Compare
| @@ -92,11 +91,12 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE | |||
| if !api.ValidNamespace(ctx, &pod.ObjectMeta) { | |||
| return nil, errors.NewConflict("pod", pod.Namespace, fmt.Errorf("Pod.Namespace does not match the provided context")) | |||
| } | |||
| pod.DesiredState.Manifest.UUID = util.NewUUID().String() | |||
| api.FillObjectMetaSystemFields(ctx, &pod.ObjectMeta) | |||
There was a problem hiding this comment.
This was called already on line 104. Please remove that call.
There was a problem hiding this comment.
Good catch. Pushed. Rebuilding.
markturansky
force-pushed
the
v1beta3_podrefactor
branch
2 times, most recently
from
88b51fd to
fcf27f1
Compare
|
All changes made. Green light. |
|
Did you get a local e2e run?
|
|
It just finished and it just failed: I'll look into it. |
|
Those tend to fail for me in vagrant. If you have a gce account (the free one should cover you) you may want to try that.
|
|
David Eads cannot run e2e either. He's running against master and gets the same errors. I don't think the failure above is from me. |
|
LGTM I'll look to merge this today after Brian signs off on it.
|
markturansky
force-pushed
the
v1beta3_podrefactor
branch
from
fcf27f1 to
8af4ccb
Compare
|
Rebased, retested, and rebuilt. Good to go. |
|
Thanks. LGTM, but my eyes are glazing over. I think we should go ahead and deal with fallout, if any. |
bgrant0607
added
the
lgtm
|
same here @bgrant0607. I'm standing by to fix any errors that may arise. |
Refactor internal API for Pods to match v1beta3
|
Incoming |
|
@bgrant0607 @abhgupta I am on it. |
|
see #2503 |
|
I'm not sure we want to change these yet, since v1beta3 isn't public. Watch field names should be versioned just like fields (validation errors too, but that's not a short term goal). ----- Original Message -----
|
…erry-pick-2095-to-release-4.17 [release-4.17] OCPBUGS-42427: UPSTREAM: 125398: Fix issue with scheduler failing on hostname mismatch
No description provided.