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.

Sign up for GitHub

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

Improve the API generated docs for managedBy #565

Merged
merged 1 commit into from
May 10, 2024
Merged

Improve the API generated docs for managedBy #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion api/jobset/v1alpha2/jobset_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,18 @@ type JobSetSpec struct {
// Suspend suspends all running child Jobs when set to true.
Suspend *bool `json:"suspend,omitempty"`

// ManagedBy is used to indicate the controller or entity that manages a JobSet
// ManagedBy is used to indicate the controller or entity that manages a JobSet.
// The built-in JobSet controller reconciles JobSets which don't have this
// field at all or the field value is the reserved string
// `jobset.sigs.k8s.io/jobset-controller`, but skips reconciling JobSets
// with a custom value for this field.
//
// The value must be a valid domain-prefixed path (e.g. acme.io/foo) -
// all characters before the first "/" must be a valid subdomain as defined
// by RFC 1123. All characters trailing the first "/" must be valid HTTP Path
// characters as defined by RFC 3986. The value cannot exceed 63 characters.
// The field is immutable.
// +optional
ManagedBy *string `json:"managedBy,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

This is just curious. Why we don't use CEL validation instead of webhook validation here?

jobset/pkg/webhooks/jobset_webhook.go

Line 231 in 7752a36

errs = append(errs, apivalidation.ValidateImmutableField(mungedSpec.ManagedBy, oldJS.Spec.ManagedBy, field.NewPath("spec").Child("managedBy"))...)

Copy link
Contributor

Choose a reason for hiding this comment

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

No specific reason for using webhook validation here. We can look into migrating to CEL as a follow up.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Thank you.


// TTLSecondsAfterFinished limits the lifetime of a JobSet that has finished
Expand Down
2 changes: 1 addition & 1 deletion api/jobset/v1alpha2/openapi_generated.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions config/components/crd/bases/jobset.x-k8s.io_jobsets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,19 @@ spec:
- message: Value is immutable
rule: self == oldSelf
managedBy:
description: ManagedBy is used to indicate the controller or entity
that manages a JobSet
description: |-
ManagedBy is used to indicate the controller or entity that manages a JobSet.
The built-in JobSet controller reconciles JobSets which don't have this
field at all or the field value is the reserved string
`jobset.sigs.k8s.io/jobset-controller`, but skips reconciling JobSets
with a custom value for this field.


The value must be a valid domain-prefixed path (e.g. acme.io/foo) -
all characters before the first "/" must be a valid subdomain as defined
by RFC 1123. All characters trailing the first "/" must be valid HTTP Path
characters as defined by RFC 3986. The value cannot exceed 63 characters.
The field is immutable.
type: string
network:
description: Network defines the networking options for the jobset.
Expand Down
2 changes: 1 addition & 1 deletion hack/python-sdk/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"$ref": "#/definitions/jobset.v1alpha2.FailurePolicy"
},
"managedBy": {
"description": "ManagedBy is used to indicate the controller or entity that manages a JobSet",
"description": "ManagedBy is used to indicate the controller or entity that manages a JobSet. The built-in JobSet controller reconciles JobSets which don't have this field at all or the field value is the reserved string `jobset.sigs.k8s.io/jobset-controller`, but skips reconciling JobSets with a custom value for this field.\n\nThe value must be a valid domain-prefixed path (e.g. acme.io/foo) - all characters before the first \"/\" must be a valid subdomain as defined by RFC 1123. All characters trailing the first \"/\" must be valid HTTP Path characters as defined by RFC 3986. The value cannot exceed 63 characters. The field is immutable.",
"type": "string"
},
"network": {
Expand Down
2 changes: 1 addition & 1 deletion sdk/python/docs/JobsetV1alpha2JobSetSpec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ JobSetSpec defines the desired state of JobSet
Name | Type | Description | Notes
------------ | ------------- | ------------- | -------------
**failure_policy** | [**JobsetV1alpha2FailurePolicy**](JobsetV1alpha2FailurePolicy.md) | | [optional]
**managed_by** | **str** | ManagedBy is used to indicate the controller or entity that manages a JobSet | [optional]
**managed_by** | **str** | ManagedBy is used to indicate the controller or entity that manages a JobSet. The built-in JobSet controller reconciles JobSets which don't have this field at all or the field value is the reserved string `jobset.sigs.k8s.io/jobset-controller`, but skips reconciling JobSets with a custom value for this field. The value must be a valid domain-prefixed path (e.g. acme.io/foo) - all characters before the first \"/\" must be a valid subdomain as defined by RFC 1123. All characters trailing the first \"/\" must be valid HTTP Path characters as defined by RFC 3986. The value cannot exceed 63 characters. The field is immutable. | [optional]
**network** | [**JobsetV1alpha2Network**](JobsetV1alpha2Network.md) | | [optional]
**replicated_jobs** | [**list[JobsetV1alpha2ReplicatedJob]**](JobsetV1alpha2ReplicatedJob.md) | ReplicatedJobs is the group of jobs that will form the set. | [optional]
**startup_policy** | [**JobsetV1alpha2StartupPolicy**](JobsetV1alpha2StartupPolicy.md) | | [optional]
Expand Down
4 changes: 2 additions & 2 deletions sdk/python/jobset/models/jobset_v1alpha2_job_set_spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def failure_policy(self, failure_policy):
def managed_by(self):
"""Gets the managed_by of this JobsetV1alpha2JobSetSpec. # noqa: E501

ManagedBy is used to indicate the controller or entity that manages a JobSet # noqa: E501
ManagedBy is used to indicate the controller or entity that manages a JobSet. The built-in JobSet controller reconciles JobSets which don't have this field at all or the field value is the reserved string `jobset.sigs.k8s.io/jobset-controller`, but skips reconciling JobSets with a custom value for this field. The value must be a valid domain-prefixed path (e.g. acme.io/foo) - all characters before the first \"/\" must be a valid subdomain as defined by RFC 1123. All characters trailing the first \"/\" must be valid HTTP Path characters as defined by RFC 3986. The value cannot exceed 63 characters. The field is immutable. # noqa: E501

:return: The managed_by of this JobsetV1alpha2JobSetSpec. # noqa: E501
:rtype: str
Expand All @@ -123,7 +123,7 @@ def managed_by(self):
def managed_by(self, managed_by):
"""Sets the managed_by of this JobsetV1alpha2JobSetSpec.

ManagedBy is used to indicate the controller or entity that manages a JobSet # noqa: E501
ManagedBy is used to indicate the controller or entity that manages a JobSet. The built-in JobSet controller reconciles JobSets which don't have this field at all or the field value is the reserved string `jobset.sigs.k8s.io/jobset-controller`, but skips reconciling JobSets with a custom value for this field. The value must be a valid domain-prefixed path (e.g. acme.io/foo) - all characters before the first \"/\" must be a valid subdomain as defined by RFC 1123. All characters trailing the first \"/\" must be valid HTTP Path characters as defined by RFC 3986. The value cannot exceed 63 characters. The field is immutable. # noqa: E501

:param managed_by: The managed_by of this JobsetV1alpha2JobSetSpec. # noqa: E501
:type: str
Expand Down