Updating usage of the --metrics flag to create a job #12174
Conversation
|
|
||
| "github.com/openshift/origin/pkg/bootstrap/docker/errors" | ||
| kbatch "k8s.io/kubernetes/pkg/apis/batch" | ||
| "time" |
There was a problem hiding this comment.
Looks like left over from previous change
| // Create the job client | ||
| jobClient := kubeClient.Batch().Jobs(infraNamespace) | ||
|
|
||
| // Create the job | ||
| deployerPod := metricsDeployerPod(hostName, imagePrefix, imageVersion) |
There was a problem hiding this comment.
you should rename the function to metricsDeployerJob and return a Job instead of a Pod
|
@csrwng Code updated, thanks! |
|
|
||
| meta := kapi.ObjectMeta{ | ||
| Name: pod.Name, | ||
| Namespace: infraNamespace, |
There was a problem hiding this comment.
you don't need to set a namespace. The client will create it in the right namespace
| @@ -186,7 +187,27 @@ func metricsDeployerPod(hostName, imagePrefix, imageVersion string) *kapi.Pod { | |||
| Env: env, | |||
| }, | |||
| } | |||
| return pod | |||
|
|
|||
There was a problem hiding this comment.
You really don't need a pod object anymore (https://github.com/openshift/origin/pull/12174/files#diff-8f476646dd3c6f6850aa06d1673053d7L147)
you should just create a pod spec and directly use it below
|
@csrwng updated |
| deadline := int64(60 * 5) | ||
|
|
||
| meta := kapi.ObjectMeta{ | ||
| Name: metricsDeployerPodName, |
There was a problem hiding this comment.
rename constant to metricsDeployerJobName
| "github.com/openshift/origin/pkg/cmd/util/clientcmd" | ||
| kapi "k8s.io/kubernetes/pkg/api" | ||
| apierrors "k8s.io/kubernetes/pkg/api/errors" | ||
|
|
||
| "github.com/openshift/origin/pkg/bootstrap/docker/errors" | ||
| kbatch "k8s.io/kubernetes/pkg/apis/batch" |
There was a problem hiding this comment.
see if you can turn off the editor automatically rearranging the imports.
We really should have something like:
"fmt"
kapi "k8s.io/kubernetes/pkg/api"
apierrors "k8s.io/kubernetes/pkg/api/errors"
kbatch "k8s.io/kubernetes/pkg/apis/batch"
"github.com/openshift/origin/pkg/bootstrap/docker/errors"
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
|
@csrwng sounds good to me, code has been updated. |
|
LGTM [merge] |
|
[Test]ing while waiting on the merge queue |
|
flake: #8571 |
|
re[test] |
|
flake #8502 |
|
flake #9203 |
|
[merge] |
|
flake #12236 |
Updated the --metrics flag of the oc cluster up command to create a job so that it will automatically retry if it fails due to the service account token not being created yet. Fixes issue #11946
|
[merge] |
|
flake #12184 |
|
[test] |
|
Evaluated for origin merge up to a340df7 |
|
Evaluated for origin test up to a340df7 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12467/) (Base Commit: 7061e0b) |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12473/) (Base Commit: 26f76ea) (Image: devenv-rhel7_5562) |
Updated the --metrics flag of the oc cluster up command to create a job
so that it will automatically retry if it fails due to the service account
token not being created yet.
Fixes issue #11946