Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement registration and listening of crd trainingjob #565

Closed
wants to merge 6 commits into from

Conversation

m3ngyang
Copy link
Member

@m3ngyang m3ngyang commented Jan 27, 2018

I have implemented a custom resource definition for paddle training job. The changes in this PR include:

  1. go/pkg/controller, responsible for the registration and listening for CRD TrainingJob, and creating a job tracker for each training job to manage its whole lifecycle
  2. go/pkg/client, auto generated codes for CRD client and informer
  3. go/hack, use our own boilerplate file as default rights comment in generated codes
  4. go/cmd/operator, a new binary component for CRD, which will be combined with the autoscaler in the near feature

About 1st change, I leave a interface for @qizheng09 to implement the job tracker/controller.

trainingjobSynced cache.InformerSynced

// TODO jobtracker keep track of every training job
jobtracker map[string]interface{}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the interface will be replaced with specific type for managing training job.

if !ok {
glog.Infof("create a new job tracker, key: '%s'", key)
glog.Infof("received job: %+v", job)
// TODO create a tracker for the job, just record its name here
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO create a tracker for the job here

@typhoonzero
Copy link
Collaborator

  1. This PR is too large to review. Or may take too long. We should keep each PR small so that we can merge it fast.
  2. Why we need a new directory pkg? If we should have one it's better to rename the top level go to pkg.
  3. Better to not have hack directory, if it's some hacking code that messing up the code, it should be very carefully reviewed.

Anyway, can you please split this PR to several small PRs so that I can review.

@m3ngyang
Copy link
Member Author

Thanks for your advice, I will split the pr and submit one by one.

@m3ngyang
Copy link
Member Author

m3ngyang commented Feb 4, 2018

I will split the pr into three parts:

  1. reorganize the directory, rename the go dir to pkg and move the dirs under current pkg dir
  2. supplement the generated codes for crd
  3. add the implementation of management of crd

Is this reasonable? @typhoonzero

@typhoonzero
Copy link
Collaborator

@m3ngyang Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants