Add condition logic code #504
Conversation
|
Fixes #502. |
|
/assign @gaocegege |
pkg/controller/controller.go
Outdated
| newTFJob := tfjob.DeepCopy() | ||
| now := metav1.Now() | ||
| newTFJob.Status.StartTime = &now | ||
| err := tc.updateTFJobConditions(newTFJob, tfv1alpha2.TFJobCreated, tfJobCreatedReason, msg) |
There was a problem hiding this comment.
LGTM but maybe I will change the code here for test purpose. Handler based implementation is more test friendly
There was a problem hiding this comment.
Oh my fault, LGTM without any comment.
There was a problem hiding this comment.
Oh my fault, LGTM without any comment.
| func (tc *TFJobController) updateTFJobConditions(tfjob *tfv1alpha2.TFJob, conditionType tfv1alpha2.TFJobConditionType, reason, message string) error { | ||
| condition := newCondition(conditionType, reason, message) | ||
| setCondition(&tfjob.Status, condition) | ||
| err := tc.updateStatusHandler(tfjob) |
There was a problem hiding this comment.
updateStatusHandler will be called in reconcile then is it necessary here?
|
Generally LGTM! Thanks for your contribution! @yph152 PTAL |
|
generally LGTM!Thanks for your contribution! |
|
pkg/controller/controller.go:67:2:warning: unused variable or constant tfJobFailedReason (varcheck) |
ScorpioCPH
force-pushed
the
v1alpha2-condition
branch
from
5d97a68
to
b965914
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing 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 |
Hi, this PR add the condition logic without UT.
@jlewi @gaocegege @yph152 @DjangoPeng @ddysher PTAL, thank!
This change is