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

implemented registration of CRDs #33

Merged
merged 1 commit into from
May 16, 2024
Merged

implemented registration of CRDs #33

merged 1 commit into from
May 16, 2024

Conversation

dmitsh
Copy link
Collaborator

@dmitsh dmitsh commented May 13, 2024

In the existing setup, each job submission requires us to define the resource's GVR (Group, Version, Resource), template, name format, and the format for pod names. These parameters remain constant.

Currently, if we have multiple "SubmitObj" tasks, we must repeat these parameters for each task.

I suggest we introduce a new task type called "RegisterObj." This task would store all the necessary parameters for a specific CRD in memory. When a job involving that CRD is submitted, the stored parameters would automatically be applied.

@dmitsh dmitsh marked this pull request as draft May 13, 2024 04:01
@dmitsh dmitsh added the wip Work in progress label May 13, 2024
@lalitadithya
Copy link
Collaborator

@dmitsh , please correct me if I'm wrong, based on my understand and our older discussion, the reason why we need this change is because we cannot infer the schema.GroupVersionResource from the group, version, kind from the YAML. The reason being that the resource does not necessarily match the kind, i.e., the kind can be "Job", but the resource will be "jobs". Is this understanding correct?

I recently leant about kubectl's api-resources, and I think this might be helpful here. If we run kubectl api-resources --api-group=batch.volcano.sh -o wide, we get the following output from which we can get the resource (jobs) from the kind (Job).

NAME   SHORTNAMES   APIVERSION                  NAMESPACED   KIND   VERBS
                     CATEGORIES
jobs   vcjob,vj     batch.volcano.sh/v1alpha1   true         Job    delete,deletecollection,get,list,patch,create,update,watch

We can parse the YAML to fetch the group and version (split the api version by / to get the group and version) and then we can subsequently use this information along with the discovery package to fetch the required mapping. This would mean that the user will no longer have to specify the group version resource in the job submission schema.

What do you think?

pkg/engine/engine.go Outdated Show resolved Hide resolved
@dmitsh
Copy link
Collaborator Author

dmitsh commented May 13, 2024

@dmitsh , please correct me if I'm wrong, based on my understand and our older discussion, the reason why we need this change is because we cannot infer the schema.GroupVersionResource from the group, version, kind from the YAML. The reason being that the resource does not necessarily match the kind, i.e., the kind can be "Job", but the resource will be "jobs". Is this understanding correct?

I recently leant about kubectl's api-resources, and I think this might be helpful here. If we run kubectl api-resources --api-group=batch.volcano.sh -o wide, we get the following output from which we can get the resource (jobs) from the kind (Job).

NAME   SHORTNAMES   APIVERSION                  NAMESPACED   KIND   VERBS
                     CATEGORIES
jobs   vcjob,vj     batch.volcano.sh/v1alpha1   true         Job    delete,deletecollection,get,list,patch,create,update,watch

We can parse the YAML to fetch the group and version (split the api version by / to get the group and version) and then we can subsequently use this information along with the discovery package to fetch the required mapping. This would mean that the user will no longer have to specify the group version resource in the job submission schema.

What do you think?

Great suggestion, thanks! Let me give it a try.
Implemented as suggested.

@dmitsh dmitsh force-pushed the ds-reg-obj branch 5 times, most recently from be2b9e7 to 2f8656e Compare May 14, 2024 03:48
@dmitsh dmitsh requested a review from lalitadithya May 14, 2024 04:41
@dmitsh dmitsh force-pushed the ds-reg-obj branch 3 times, most recently from 4b6a6c8 to 176a006 Compare May 15, 2024 21:18
@dmitsh dmitsh marked this pull request as ready for review May 15, 2024 21:19
@dmitsh dmitsh force-pushed the ds-reg-obj branch 2 times, most recently from de9ccae to c624dc7 Compare May 16, 2024 01:44
@dmitsh dmitsh requested a review from lalitadithya May 16, 2024 01:48
@dmitsh dmitsh force-pushed the ds-reg-obj branch 3 times, most recently from 00994c9 to e0b71c3 Compare May 16, 2024 05:20
@dmitsh dmitsh removed the wip Work in progress label May 16, 2024
pkg/engine/check_pod_task.go Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Shmulevich <[email protected]>
@dmitsh dmitsh merged commit 7de87db into main May 16, 2024
4 checks passed
@dmitsh dmitsh deleted the ds-reg-obj branch May 16, 2024 11:09
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