Clean up code for custom CRD#1355
Conversation
178c775 to
364df94
Compare
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andreyvelich, gaocegege The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@andreyvelich: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
364df94 to
00e1d9b
Compare
|
New changes are detected. LGTM label has been removed. |
I removed redundant code and fixed tests for new custom CRD implementation.
As well, I added some default values for
Job,PyTorchJobandTFJobto make examples easier for the first look, as we discussed before.In particular, for
Job:SuccessConditionandFailureCondition.I didn't add
PrimaryPodLabelstoJobexamples since eachJobhas only one pod replica. IfPrimaryPodLabelsis omitted, metrics collector injected to all created pods.For Kubeflow Job (
PytorchJobandTFJob).SuccessConditionandFailureCondition.PrimaryPodLabels.I manually added
PrimaryContainerNameto all examples. I think it is very easy to understand and user can explicitly see which container will be wrapped by metrics collector.I can't set default values if Trial Template is set in ConfigMap, since it requires k8s client. We can think about it later.
I left validation for
Job,TFJobandPyTorchJobfor now. @gaocegege @johnugeorge What do you think about it ?I deleted
job/v1beta1since it is not used. Let's continue discussion here: #1320 how we should implementMutateJob./assign @gaocegege @johnugeorge