Skip to content

[WIP] Drop lifecycle_state check in validate_reconfigure - #24006

Open
agrare wants to merge 1 commit into
ManageIQ:masterfrom
agrare:drop_lifecycle_state_provisioned_service_validate_reconfigure
Open

agrare wants to merge 1 commit into
ManageIQ:masterfrom
agrare:drop_lifecycle_state_provisioned_service_validate_reconfigure

Conversation

@agrare

@agrare agrare commented Oct 12, 2026

Copy link
Copy Markdown
Member

Adding workflows-based reconfigure to Service also added checks that the service wasn't retired/retiring or hadn't been successfully provisioned.

Adding workflows-based reconfigure to Service also added checks that the
service wasn't retired/retiring or hadn't been successfully provisioned.
@agrare
agrare requested a review from Fryguy as a code owner October 12, 2026 17:06
@agrare

agrare commented Oct 12, 2026

Copy link
Copy Markdown
Member Author

ref

I'm not sure if this should be considered a bug in the original PR.

@Fryguy wanted to revisit this since you mentioned it might be a bug. I could see return false if !(provisioned? || provision_failed?) being a little too aggressive, but not allowing reconfigure on a retired service seems reasonable?

Do we want to allow reconfigure of a service which is currently being provisioned, or has failed to provision successfully?

@Fryguy Fryguy added the wip label Oct 14, 2026
@miq-bot

miq-bot commented Oct 14, 2026

Copy link
Copy Markdown
Member

Checked commit agrare@68e0f0c with ruby 3.3.12, rubocop 1.91.0, haml-lint 0.78.0, and yamllint 1.37.1
1 file checked, 0 offenses detected
Everything looks fine. 👍

@Fryguy

Fryguy commented Oct 23, 2026

Copy link
Copy Markdown
Member

Reading back, I'm honestly not sure what I was thinking. I think I was saying that this seemed to have introduced a "requirement" on lifecycle_state that didn't exist previously. If we expect that, then maybe we should have a non-null validation. or an enum validation on the column. Or, we code into the method an expectation of null and what that means. Either way, it feels like something is missing somewhere.

@agrare

agrare commented Oct 23, 2026

Copy link
Copy Markdown
Member Author

Hm there isn't a new requirement on lifecycle_state necessarily, just that you can't reconfigure a service that is e.g. retired or not provisioned yet. The previous tests were assuming that you could reconfigure a service in any state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants