-
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
Store Jobs under batch/v1 instead of deprecated extensions/v1beta1 #12517
Conversation
[test] |
@soltysh I read the title as "Steve Jobs under batch/v1... " :-) |
// } | ||
|
||
func TestStorageVersionsUnified(t *testing.T) { | ||
func TestStorageVersions(t *testing.T) { |
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.
do we have a test to make sure that an extensions/v1beta1 Job can be read from storage, and if updated, gets persisted back as a batch/v1 Job?
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.
Yes, down this file:
origin/test/integration/storage_versions_test.go
Lines 148 to 153 in b7dfdfe
if _, err := projectAdminKubeClient.Batch().Jobs(ns).Get(job.Name); err != nil { | |
t.Errorf("%s: Error reading Job from the batch client: %#v", name, err) | |
} | |
if _, err := projectAdminKubeClient14.Extensions().Jobs(ns).Get(job.Name); err != nil { | |
t.Errorf("%s: Error reading Job from the extensions client: %#v", name, err) | |
} |
@@ -184,7 +184,7 @@ func BuildDefaultAPIServer(options configapi.MasterConfig) (*apiserveroptions.Se | |||
master.DefaultAPIResourceConfigSource(), | |||
)*/ | |||
// the order here is important, it defines which version will be used for storage | |||
storageFactory.AddCohabitatingResources(extensions.Resource("jobs"), batch.Resource("jobs")) | |||
storageFactory.AddCohabitatingResources(batch.Resource("jobs"), extensions.Resource("jobs")) | |||
storageFactory.AddCohabitatingResources(extensions.Resource("horizontalpodautoscalers"), autoscaling.Resource("horizontalpodautoscalers")) |
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.
@DirectXMan12 should the same storage version switch be done for HPA? I assume we'll want a similar test (store as extensions/v1beta1 HPA, read/write and ensure the update was stored as autoscaling/v1 HPA)
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.
HPA doesn't have generated client for extensions/v1beta1, that's why we're missing those.
e5863a7
to
c601e5f
Compare
Flake #11024. [test] |
Flake #12540 [test] |
Grrr.... the failures are playing on my nerves today. I've rebased the PR and waiting for fresh results. I hit #12540 once again. |
I can't reproduce #12558 manually locally nor in AWS, trying in this PR. |
Yikes, panic on in cluster config? |
That's just me chasing flakes, nothing to worry about. |
87b3bb4
to
626690c
Compare
I've cherry-picked the fix from #12582 here and will re-run tests. That will give me double check for this error :) |
I've cherry-picked the new version and let the fun begin with a new test run... |
Evaluated for origin test up to 3d343d5 |
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13089/) (Base Commit: bb1a153) |
continuous-integration/openshift-jenkins/test SUCCESS |
@liggitt anything else left here or this is good to merge? |
LGTM |
[merge] |
Evaluated for origin merge up to 3d343d5 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13173/) (Base Commit: 389335e) (Image: devenv-rhel7_5751) |
Fixes #10950. This only changes the version under jobs are stored in etcd, the location stays the same.
@deads2k @liggitt ptal