* feat(api): add progress types to api
Signed-off-by: Rob Bell <[email protected]>
* feat: progress server stub
Signed-off-by: Rob Bell <[email protected]>
* feat: setup tls for server
Signed-off-by: Rob Bell <[email protected]>
* refactor: pull out middleware
Signed-off-by: Rob Bell <[email protected]>
* refactor: remove logger message noise from tests
Signed-off-by: Rob Bell <[email protected]>
* fix: send server error logs to main logger
Signed-off-by: Rob Bell <[email protected]>
* feat: update the trainer status
Signed-off-by: Rob Bell <[email protected]>
* feat: use separate client to avoid impacting main reconciler client-side rate limits
Signed-off-by: Rob Bell <[email protected]>
* feat: configure progress server in helm chart and configmap
Signed-off-by: Rob Bell <[email protected]>
* fix: prevent race condition
Signed-off-by: Rob Bell <[email protected]>
* feat: inject progress runtime config
Signed-off-by: Rob Bell <[email protected]>
* fix: progress client config
Signed-off-by: Rob Bell <[email protected]>
* fix: update generated config
Signed-off-by: Rob Bell <[email protected]>
* feat: validate auth tokens in endpoint
Signed-off-by: Rob Bell <[email protected]>
* test: validate auth tokens in endpoint
Signed-off-by: Rob Bell <[email protected]>
* refactor: tidy tests to use same format as other tests
Signed-off-by: Rob Bell <[email protected]>
* fix: look up the cluster issuer from the service account token
Different clusters can use different issuers so look up the issuer from the trusted in-cluster service account token.
Signed-off-by: Rob Bell <[email protected]>
* refactor: rename authn middleware
Signed-off-by: Rob Bell <[email protected]>
* feat: add authn to status update endpoint
Signed-off-by: Rob Bell <[email protected]>
* refactor: tidy up reading in-cluster service account token
Signed-off-by: Rob Bell <[email protected]>
* fix: rbac permissions
Signed-off-by: Rob Bell <[email protected]>
* fix: better error response
Signed-off-by: Rob Bell <[email protected]>
* refactor: move kubebuilder annotation higher up
Signed-off-by: Rob Bell <[email protected]>
* fix: make lastUpdateTime required
Signed-off-by: Rob Bell <[email protected]>
* chore: fix config tests after rebase
Signed-off-by: Rob Bell <[email protected]>
* test: update config test with new progress server config
Signed-off-by: Rob Bell <[email protected]>
* test: add progress tracking e2e test
Signed-off-by: Rob Bell <[email protected]>
* fix: allow feature gates to be configurable from command line argument
So they can be overridden in e2e tests
Signed-off-by: Rob Bell <[email protected]>
* refactor: reuse shared progress StatusUrl
Signed-off-by: Rob Bell <[email protected]>
* test: tidy up tests
Signed-off-by: Rob Bell <[email protected]>
* refactor: allow plugins to use config
Signed-off-by: Rob Bell <[email protected]>
* refactor: centralise shared constants
Signed-off-by: Rob Bell <[email protected]>
* fix: disable http2 in progress server tls opts if disabled by config
Signed-off-by: Rob Bell <[email protected]>
* fix: minor improvements suggested by copilot
Signed-off-by: Rob Bell <[email protected]>
* chore: rename progress server => status server
Signed-off-by: Rob Bell <[email protected]>
* chore: update type doc
Signed-off-by: Rob Bell <[email protected]>
* chore: update name of feature gate
Signed-off-by: Rob Bell <[email protected]>
* fix: after rebase
Signed-off-by: Rob Bell <[email protected]>
* chore: fix typo
Signed-off-by: Rob Bell <[email protected]>
* chore: rename handler
Signed-off-by: Rob Bell <[email protected]>
* fix: run status server on all replicas
Signed-off-by: Rob Bell <[email protected]>
* fix: deep copy the rest config
Signed-off-by: Rob Bell <[email protected]>
* refactor: use ssa
Signed-off-by: Rob Bell <[email protected]>
* refactor: separate token audience per train job
Signed-off-by: Rob Bell <[email protected]>
* chore: rename status server
Signed-off-by: Rob Bell <[email protected]>
* fix: revert adding status pkg to manifest gen paths
Signed-off-by: Rob Bell <[email protected]>
* chore: update log messages
Signed-off-by: Rob Bell <[email protected]>
* chore: update api docs
Signed-off-by: Rob Bell <[email protected]>
* chore: update format
Signed-off-by: Rob Bell <[email protected]>
* fix: revert inadvertent change
Signed-off-by: Rob Bell <[email protected]>
* chore: update api name and type docs
Signed-off-by: Rob Bell <[email protected]>
* refactor: initialise oidc provider in manager startup
Signed-off-by: Rob Bell <[email protected]>
* fix: allow empty update request
Signed-off-by: Rob Bell <[email protected]>
* fix: wait for server shut down
Signed-off-by: Rob Bell <[email protected]>
* fix: handle missing train job
Signed-off-by: Rob Bell <[email protected]>
* refactor: update env var names
Signed-off-by: Rob Bell <[email protected]>
* chore: rename packages, plugins, feature gate
Signed-off-by: Rob Bell <[email protected]>
* refactor: update env var names
Signed-off-by: Rob Bell <[email protected]>
* chore: regenerate after rebase
Signed-off-by: Rob Bell <[email protected]>
* chore: fixes after rebase
Signed-off-by: Rob Bell <[email protected]>
* chore: minor doc updates
Signed-off-by: Rob Bell <[email protected]>
* chore: rename status server config
Signed-off-by: Rob Bell <[email protected]>
* chore: replace placeholder config with nil
Signed-off-by: Rob Bell <[email protected]>
* chore: revert separate runtime utils package
Signed-off-by: Rob Bell <[email protected]>
* chore: remove unused config from tests
Signed-off-by: Rob Bell <[email protected]>
---------
Signed-off-by: Rob Bell <[email protected]>
* feat(api): add ActiveDeadlineSeconds and TTLSecondsAfterFinished to Trainer APIs
- Add ActiveDeadlineSeconds *int64 to TrainJobSpec (immutable, min=1)
with reason constant TrainJobDeadlineExceededReason="DeadlineExceeded"
- Add TTLSecondsAfterFinished *int32 to TrainingRuntimeSpec (min=0)
with cross-field CEL rules blocking conflicting lifecycle fields
in the JobSet/Job template
- Add TrainJobDeadlineExceededMessage to constants
- Regenerate zz_generated.deepcopy.go
Part of KEP-2899
Signed-off-by: XploY04 <[email protected]>
* feat(controller): add TTL-based auto-deletion for finished TrainJobs
- Add early-exit guard in Reconcile before prevTrainJob DeepCopy and
removeFailedCondition, mirroring the Kubernetes Job controller pattern.
Once a TrainJob is terminal (Complete or Failed), only TTL cleanup runs.
- Add fetchTTLFromRuntime helper to retrieve TTLSecondsAfterFinished from
the referenced TrainingRuntime or ClusterTrainingRuntime.
- Add reconcileTTL helper to delete finished TrainJobs after their TTL
expires, or requeue at the exact expiry time.
Part of KEP-2899
Signed-off-by: XploY04 <[email protected]>
* feat(controller): enforce ActiveDeadlineSeconds on TrainJobs
- Add reconcileDeadline helper to terminate running TrainJobs that
exceed their ActiveDeadlineSeconds.
- Handle job suspension: StartTime is determined by the LastTransitionTime
of the Suspended=False condition (if it exists) to ensure the deadline
timer resets correctly upon resume.
- Jobs exceeding the deadline are moved to a Failed state with
reason DeadlineExceeded.
Part of KEP-2899
Signed-off-by: XploY04 <[email protected]>
* feat(webhooks): warn on short ttlSecondsAfterFinished
- Add an admission warning to both TrainingRuntime and
ClusterTrainingRuntime webhooks if the TTLSecondsAfterFinished
is configured to less than 60 seconds, as this may cause
unexpected data/log loss for data scientists due to overly
aggressive cleanup.
Part of KEP-2899
Signed-off-by: XploY04 <[email protected]>
* test: add integration tests for TrainJob TTL and deadline lifecycle
- Add integration tests for DeadlineExceeded behavior in TrainJob controller
- Add integration tests for TTL validation and warnings in webhooks
Part of KEP-2899
Signed-off-by: XploY04 <[email protected]>
* cleanup
Signed-off-by: XploY04 <[email protected]>
* fix: ensure JobSet is suspended on expiration and fix webhook test regress
Signed-off-by: XploY04 <[email protected]>
* fix(controller): return errors from fetchTTLFromRuntime for proper requeue
Signed-off-by: XploY04 <[email protected]>
* test: remove unused wantWarning from webhook tests
Signed-off-by: XploY04 <[email protected]>
* fix
Signed-off-by: XploY04 <[email protected]>
* feat: Remove from TrainingRuntime and ClusterTrainingRuntime specifications and associated validation.
Signed-off-by: XploY04 <[email protected]>
* fix(api): restore `activeDeadlineSeconds` validation rule on TrainingRuntime
Signed-off-by: XploY04 <[email protected]>
* refactor: remove unnecessary blank lines and improve logging in ClusterTrainingRuntime validation
Signed-off-by: XploY04 <[email protected]>
* feat: add activeDeadlineSeconds field to TrainJobSpec and update related tests
Signed-off-by: XploY04 <[email protected]>
* Remove unnecessary newline in test file
Signed-off-by: Yash Agarwal <[email protected]>
* Remove unnecessary newline in trainingruntime_webhook_test.go
Signed-off-by: Yash Agarwal <[email protected]>
* refactor: remove unnecessary blank lines in TrainJob reconciliation and deadline tests
Signed-off-by: XploY04 <[email protected]>
* feat: change ActiveDeadlineSeconds from pointer to value type and update related code
Signed-off-by: XploY04 <[email protected]>
* feat: add condition check for TrainJob deadline exceeded before suspending
Signed-off-by: XploY04 <[email protected]>
* feat: enhance TrainJob deadline handling and suspend logic
Signed-off-by: XploY04 <[email protected]>
* feat: enhance TrainJob deadline handling and suspend logic
Signed-off-by: XploY04 <[email protected]>
* fix
Signed-off-by: XploY04 <[email protected]>
* feat: change ActiveDeadlineSeconds to pointer type and update related logic
Signed-off-by: XploY04 <[email protected]>
* feat: ensure requeueAfter is at least 1 second when deadline is exceeded
Signed-off-by: XploY04 <[email protected]>
* feat: change ActiveDeadlineSeconds from pointer to value type and update related logic
Signed-off-by: XploY04 <[email protected]>
* fix: remove early return for finished TrainJobs to prevent status sync issues
The early return was preventing the controller from syncing JobSet
status updates for finished TrainJobs, causing e2e tests to timeout
waiting for conditions that never got updated.
This fix restores the upstream behavior where finished TrainJobs still
go through status reconciliation (though reconcileObjects is skipped).
Signed-off-by: XploY04 <[email protected]>
* fix
Signed-off-by: XploY04 <[email protected]>
* fix: remove x-kubernetes-validations for activeDeadlineSeconds in TrainingRuntime CRDs
Signed-off-by: XploY04 <[email protected]>
* fix: update validation message for ActiveDeadlineSeconds to be more generic
Signed-off-by: XploY04 <[email protected]>
* replacing suspend with delete
Signed-off-by: XploY04 <[email protected]>
* fix: remove SpecLabel and SpecAnnotation methods from TrainJobWrapper
Signed-off-by: XploY04 <[email protected]>
* fix: remove annotations field from TrainJobSpec and CRD definitions
Signed-off-by: XploY04 <[email protected]>
* ensure JobSet deletion in e2e tests
Signed-off-by: XploY04 <[email protected]>
* fix: add JobSet APIs to the scheme in e2e tests
Signed-off-by: XploY04 <[email protected]>
* fix: simplify error handling in Reconcile method of TrainJobReconciler
Signed-off-by: XploY04 <[email protected]>
---------
Signed-off-by: XploY04 <[email protected]>
Signed-off-by: Yash Agarwal <[email protected]>
* Fix bug with jobs not being marked as completed.
* A bug was introduced with getting the replica status in #344 which
switched to creating pods directly.
* Our presubmits/postsubmits were failing but this went unnoticed because
the git status check was improperly reported as succeeded.
* The bug is because we try to get the pod status by name but the name
doesn't include the random salt in the pod name.
* The code in question is a legacy of when we were using job controllers and
we first got the status of the job controller. We incorrectly changed that
code to get the pod. The correct thing is to just list pods by label; we
already do that in the code below so we just need to delete some code.
* Don't create any resources if the DeletionTimestamp is set.
Creating resources at this point would end blocking deletion of the object
because the controller would create resources while we are trying to delete
them.
* Use logrus in controller.go, trainer.go, and replicas.go to log
with fields providing information about the job and repliac.
This makes it easy to filter logs for a particular job.
* Use logrus to log the name of the job in a field.
* Checking the deletiontime stamp doesn't appear to be sufficient.
Use the Phase to determine whether we should create resources.
* Run gofmt.
* * Reset the rate limiter after every successful sync.
* Otherwise the ratelimiter will end up delaying processing subsequent
events which isn't what we want.
* Run goimports to fix lint issues.
* * Reconcile needs to update the TFJob stored in TrainingJob. This ensures
TrainingJob has an up to date representation of the job.
* Otherwise changes made to the spec won't be available to TrainingJob. For
example, if the job is deleted by the user, the deletion timestamp will
be set. But if we don't update the TFJob stored in TrainingJob this
change won't be propogated.
* * TrainingJob.update should log the value of the job not the pointer.
* Add more comments to the code.
* Create PDB of TFReplicaSet for gang scheduling by kube-arbitrator
kube-arbitrator (technically its component kube-batchd) requires PDB
for its gang scheduling feature. The feature is useful for creating
every pod of TFJob at the same time. This commit lets tf-operator
create the PDB for the purpose.
* Add a new UT TestPDBForGangScheduling for checking PDB creation
* Move from glog to sirupsen/logrus for logging
* Add a new flag json_log_format
* Refactor all log statements to use logrus
* Use Info level for log level V(1) and below
* Use Debug level for log level V(2) and above
* Tested locally
Addresses https://github.com/kubeflow/tf-operator/issues/24
Sample logs
```
{"filename":"app/server.go:54","level":"info","msg":"EnvKubeflowNamespace not set, use default namespace","time":"2018-02-27T18:25:18-08:00"}
{"filename":"app/server.go:59","level":"info","msg":"[Version: 0.3.0+git Git SHA: Not provided. Go Version: go1.9.3 Go OS/Arch: darwin/amd64]","time":"2018-02-27T18:25:18-08:00"}
{"filename":"app/server.go:145","level":"info","msg":"No controller_config_file provided; using empty config.","time":"2018-02-27T18:25:18-08:00"}
{"filename":"controller/controller.go:110","level":"info","msg":"Setting up event handlers","time":"2018-02-27T18:25:18-08:00"}
```
This appears to be a bug with the informer forgetting about active jobs.
I think the root cause is that delete events are processed directly
as opposed to using the work queue. This causes race conditions because
the work queue is what ensures only on thread is processing a given job
at a time.
Since we no longer explicitly handle delete events we rely on garbage
collection and owner references to delete the pods.
We add testing to our test runner to verify pods are garbage collected.
We update test_runner.py to try recreating jobs with the same name.
In general this doesn't reproduce the original bug.
Related to #322 (to need to push a new image to fix).
Fix#310
Related to #373
* In syncTfJob when checking whether a work queue item corresponds to a TrainingJob already in the map we need to check the UID. Otherwise we will not properly handle the case where a training job is deleted and then a new job is recreated with the same name.
* We need to make sure that the Replicas field in TrainingJob is always properly set;
* We were only initializing replicas in setup which was problematic in the case where the TfJob controller gets restarted because on restarted setup won't be invoked because the job is past that phase and as a result the replicas won't be reinitialized.
* test_runner needs to ignore case when checking whether the job succeeded otherwise we conclude
that successful jobs failed
* The controller should only forget about job after the job has been cleaned up; not when it is marked as succeeded or failed.
* Add back code to support termination policies use the worker and not the master as the chief
*This was added in #221 and accidentally removed in the refactor in #234.
* Add an Update function to the controller.
* The informer periodically generates an Update event but we aren't processing these because we don't have an update function.
* As a result, TrainingJob.reconcile doesn't get called periodically and we aren't properly updating the status of the job.
ref #309
ref #293
This PR is mainly about refactoring the structure of controller keep style with kubernetes controller. And the main logic of package trainer is not modified. We can continue do refactor next step to make the controller stateless.
and gradually tighten the linting rules. for now unused and golint are left out.
some fixes are included in this PR.
using gometaliner instead of just golint because:
it covers more cases, e.g. the typo checkings.
it also allows for disabling rules w.r.t regex, e.g. when you don't want to enforce all the comment rules enforced by golint
* Part of #53
Increase logging to try to track down issues with TensorBoard not being deleted #107.
Set the default image in the helm chart to the latest image
* We add the latest tag with every release
* Furthermore, when we generate releases we actually override values.yaml to pin values for certain images.
Update e2e.go so that we can potentially run multiple jobs to try to stress the operator.
* Update test infrastructure to use repo tensorflow/k8s
* Update README to refer to new repo.
* Rename go packages from jlewi/mlkube.io > tensorflow/k8s.
* Update repo location in the helm e2e test.
* Update links.
* Update the readme.md
Add an option to automatically run a standard gRPC server for parameter servers so users don't have to supply it themselves.
* Use a ConfigMap to inject a python script that can start the standard TF gRPC server
* There is one ConfigMap per TfJob; this avoids problems created by a TfJob operator upgrade making changes to the python script.
* Always check for existing TfJobs and instantiate controllers for them.
* Whenever we start the controller, we should check if there are existing
TfJobs and instantiate controllers for them so they aren't orphaned.
* Previously we only did this if the CRD already existed. IMO it seems
more robust to always do it and not rely on the CRD already existing.
* Added a log message for the watch version obtained.
