feat: add HostnamePolicy field to CollaSetSpec#50
Open
ColdsteelRail wants to merge 1 commit into
Open
Conversation
Add HostnamePolicyType and HostnamePolicyPodName constant to support configuring pod hostname based on pod name in CollaSet. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new HostnamePolicy field (with constant HostnamePolicyPodName) to CollaSetSpec so a CollaSet can declare that each pod's spec.hostname should match its pod name. The change is type-only at the moment — the Go change is limited to API definitions, and the CRD YAMLs reflect both the new field and a large amount of unrelated controller-gen/schema regeneration.
Changes:
- Introduce
HostnamePolicyTypeandHostnamePolicyPodNameconstant inapps/v1alpha1/collaset_types.go, and addHostnamePolicyfield toCollaSetSpec. - Regenerate
apps.kusionstack.io_collasets.yamlto include the newhostnamePolicyschema (plus large unrelated upstream schema updates for PVC templates). - Unrelated CRD schema regenerations in
apps.kusionstack.io_poddecorations.yamlandrollout.kusionstack.io_backendroutings.yaml(gRPC probes,resizePolicy,claims,dataSourceRef.namespace, description text refreshes, etc.).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/v1alpha1/collaset_types.go | Adds the HostnamePolicyType enum type, HostnamePolicyPodName constant, and the new HostnamePolicy field on CollaSetSpec. No consumer logic is added. |
| config/crd/apps/apps.kusionstack.io_collasets.yaml | Adds the hostnamePolicy property to the CRD; also includes unrelated PVC template schema regeneration (claims, dataSourceRef.namespace, resizeStatus, allocatedResources, etc.). |
| config/crd/apps/apps.kusionstack.io_poddecorations.yaml | Unrelated CRD regeneration: adds gRPC probe, resizePolicy, resources.claims, dataSourceRef.namespace, and description refreshes throughout the pod template schema. |
| config/crd/rollout/rollout.kusionstack.io_backendroutings.yaml | Unrelated CRD regeneration: reformats the Condition description to a single-line YAML string. |
Comments suppressed due to low confidence (2)
apps/v1alpha1/collaset_types.go:140
- The new
HostnamePolicyfield is declared in the API type and exposed in the CRD, but a search of the repository shows no consumer of this field anywhere. There is no controller logic that readsHostnamePolicyand setspod.Spec.Hostnameto the pod name, so settinghostnamePolicy: PodNameon a CollaSet will currently have no effect. As-shipped, this is a non-functional API addition. The implementation that actually applies the hostname to created pods (and any unit/e2e tests covering it) needs to be included before this field is exposed to users.
// HostnamePolicy indicates how the pod's hostname is determined.
// When set to "PodName", the pod's spec.hostname will be set to the pod name.
// This requires NamingStrategy.PodNamingSuffixPolicy to be PersistentSequence.
// +optional
HostnamePolicy HostnamePolicyType `json:"hostnamePolicy,omitempty"`
apps/v1alpha1/collaset_types.go:140
- The doc comment states "This requires NamingStrategy.PodNamingSuffixPolicy to be PersistentSequence", but there is no validation enforcing this precondition (no webhook check, no CEL rule on the CRD field, no controller-side guard). A user can set
hostnamePolicy: PodNametogether with the defaultRandomsuffix policy (or with no NamingStrategy at all), in which case pod names are generated server-side and the stated invariant is silently violated. Add an admission/validation check that rejects this combination, or document the behavior when the precondition is not met.
// HostnamePolicy indicates how the pod's hostname is determined.
// When set to "PodName", the pod's spec.hostname will be set to the pod name.
// This requires NamingStrategy.PodNamingSuffixPolicy to be PersistentSequence.
// +optional
HostnamePolicy HostnamePolicyType `json:"hostnamePolicy,omitempty"`
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+67
| // HostnamePolicyType indicates how the pod's hostname is determined. | ||
| type HostnamePolicyType string | ||
|
|
||
| const ( | ||
| // HostnamePolicyPodName sets the pod's hostname to the pod name. | ||
| // This requires namingStrategy.podNamingSuffixPolicy to be PersistentSequence, | ||
| // as the pod name must be deterministic before creation. | ||
| HostnamePolicyPodName HostnamePolicyType = "PodName" | ||
| ) |
Comment on lines
+1424
to
+1443
| grpc: | ||
| description: GRPC specifies an action involving a GRPC | ||
| port. | ||
| properties: | ||
| port: | ||
| description: Port number of the gRPC service. Number | ||
| must be in the range 1 to 65535. | ||
| format: int32 | ||
| type: integer | ||
| service: | ||
| description: |- | ||
| Service is the name of the service to place in the gRPC HealthCheckRequest | ||
| (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). | ||
|
|
||
|
|
||
| If this is not specified, the default behavior is defined by gRPC. | ||
| type: string | ||
| required: | ||
| - port | ||
| type: object |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add HostnamePolicyType and HostnamePolicyPodName constant to support configuring pod hostname based on pod name in CollaSet.
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
2. What is the scope of this PR (e.g. component or file name):
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
6. Release note
Please refer to Release Notes Language Style Guide to write a quality release note.