Skip to content

Migrate Scale.spec.replicas to declarative#141

Merged
thockin merged 7 commits intovalidation-genfrom
subresources-enable-replicas
Apr 14, 2025
Merged

Migrate Scale.spec.replicas to declarative#141
thockin merged 7 commits intovalidation-genfrom
subresources-enable-replicas

Conversation

@jpbetz
Copy link
Copy Markdown
Owner

@jpbetz jpbetz commented Mar 17, 2025

Builds on #140

This sets up the wiring for replicationcontroller/scale and provides and example of how to do it in general.

Note that this does not re-evaluate the pre-existing decision that scale subresource be double validated-- once against scale subresource validation and then again against containing resource validation.

Testing in the storage package is not ideal. Suggestion welcome.

@jpbetz jpbetz force-pushed the subresources-enable-replicas branch 4 times, most recently from 0c84b68 to 866e377 Compare March 20, 2025 03:00
@jpbetz jpbetz changed the title [DNM][WIP] Migrate Scale's spec.replicas to declarative Migrate Scale.spec.replicas to declarative Mar 20, 2025
@yongruilin yongruilin force-pushed the validation-gen branch 3 times, most recently from cdbf203 to d78db0f Compare March 26, 2025 20:57
@jpbetz jpbetz force-pushed the subresources-enable-replicas branch from 866e377 to 21f21b6 Compare March 27, 2025 01:35
@jpbetz
Copy link
Copy Markdown
Owner Author

jpbetz commented Mar 27, 2025

Thanks @yongruilin feedback applied.

@jpbetz jpbetz force-pushed the subresources-enable-replicas branch 2 times, most recently from 05110ba to 84f5165 Compare March 29, 2025 12:41
@jpbetz
Copy link
Copy Markdown
Owner Author

jpbetz commented Mar 29, 2025

@thockin All feedback applied

@jpbetz jpbetz force-pushed the subresources-enable-replicas branch 3 times, most recently from ce2a7b2 to 230d392 Compare March 29, 2025 13:33
Copy link
Copy Markdown
Collaborator

@thockin thockin left a comment

Choose a reason for hiding this comment

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

I only see the new request.Subresources[] field in tests -- am I missing it, or it's just not used for real yet?

jpbetz added 7 commits April 14, 2025 18:28
This adds declarative validation for all all versioned types of Scale
since our testing uses apitesting.VerifyVersionedValidationEquivalence,
will fail if we don't convert them all at the same time.
This applies to all group version kinds of Scale.  Validation-gen must be enabled for all at the
same time since our testing checks that cross-GV declarative validation to matches.
…pec.replicas validation.

This does not change the resulting errors but it makes declarative validation testing easier.
@jpbetz jpbetz force-pushed the subresources-enable-replicas branch from 230d392 to 9e7e3e4 Compare April 14, 2025 22:47
@jpbetz
Copy link
Copy Markdown
Owner Author

jpbetz commented Apr 14, 2025

I only see the new request.Subresources[] field in tests -- am I missing it, or it's just not used for real yet?

It might be quite some time before we actually use request.Subresources because there are so few applications. Pod /resize and /ephermeralContainers will need it. I suppose I could have deferred that PR to put subresources in options, but I wanted to get it out of our validator parameter lists.

@jpbetz
Copy link
Copy Markdown
Owner Author

jpbetz commented Apr 14, 2025

Thanks @thockin. Feedback applied

@thockin thockin merged commit 39308a1 into validation-gen Apr 14, 2025
1 check passed
@jpbetz
Copy link
Copy Markdown
Owner Author

jpbetz commented Apr 30, 2025

I only see the new request.Subresources[] field in tests -- am I missing it, or it's just not used for real yet?

It's not used for real yet.

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.

3 participants