controller: Add defaulter #483
Conversation
hack/update-codegen.sh
Outdated
| @@ -28,7 +28,8 @@ CODEGEN_PKG=${CODEGEN_PKG:-$(cd ${SCRIPT_ROOT}; ls -d -1 ./vendor/k8s.io/code-ge | |||
| # --output-base because this script should also be able to run inside the vendor dir of | |||
| # k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir | |||
| # instead of the $GOPATH directly. For normal projects this can be dropped. | |||
| ${CODEGEN_PKG}/generate-groups.sh "defaulter,deepcopy,client,informer,lister" \ | |||
| # Notice: The code in code-generator does not generate defaulter by default. | |||
There was a problem hiding this comment.
Maybe add the defaulter to update-codegen.sh?
https://github.com/kubeflow/pytorch-operator/blob/master/hack/update-codegen.sh#L36
There was a problem hiding this comment.
Thanks for your advice I have asked the upstream how to deal with the defaulter-gen and if they do not reply I will add it to update-codegen
|
/assign @ScorpioCPH |
|
|
||
| defaultPortName = "default-tfjob-port" | ||
| defaultPort = 2222 | ||
| defaultImage = "tensorflow/tensorflow:1.3.0" |
There was a problem hiding this comment.
Why do we need to set a default image?
I think this is user-defined :)
There was a problem hiding this comment.
https://github.com/kubeflow/tf-operator/blob/master/pkg/apis/tensorflow/v1alpha1/defaults.go#L31
I follow the code in master, and I agree with you. I think we could remove the defaults for image.
Signed-off-by: Ce Gao <[email protected]>
Signed-off-by: Ce Gao <[email protected]>
Signed-off-by: Ce Gao <[email protected]>
Signed-off-by: Ce Gao <[email protected]>
Signed-off-by: Ce Gao <[email protected]>
|
Updated, PTAL @ScorpioCPH |
|
|
||
| func setDefaultRestartPolicy(spec *TFReplicaSpec) { | ||
| if spec.RestartPolicy == RestartPolicy("") { | ||
| spec.RestartPolicy = RestartPolicyAlways |
There was a problem hiding this comment.
If none of the restart policies is specified, kubernetes will set it to RestartPolicyAlways by default.
There was a problem hiding this comment.
Yeah but the RestartPolicy is not pod's, it is defined in replicasSpec by our own, I think we should set the default for it manually. Could k8s do it for us?
| @@ -0,0 +1,21 @@ | |||
| package v1alpha2 | |||
Signed-off-by: Ce Gao <[email protected]>
Signed-off-by: Ce Gao <[email protected]>
|
Addressed your comments, PTAL @ScorpioCPH |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ScorpioCPH The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Ce Gao [email protected]
This change is