Fixes #38270 - Don't create content view environment when CV is not published to LCE #11338
Conversation
This file contains 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
jeremylenz
force-pushed
the
38270-no-first-or-create-silly
branch
from
a4c74d0 to
6ab8066
Compare
|
Requires theforeman/hammer-cli-foreman#638 to see the hammer error. |
jeremylenz
force-pushed
the
38270-no-first-or-create-silly
branch
from
6ab8066 to
4943f41
Compare
|
I have confirmed that this is working as intended for both a valid and invalid combination of CV and LE. I need to investigate the test failures before acking, however. |
jeremylenz
commented
Mar 21, 2025
Comment on lines
171
to
173
| cv_name = ::Katello::ContentView.find_by(:id => content_view_id)&.name | ||
| fail ::Katello::Errors::ContentViewEnvironmentError, _("Unable to find a content view with ID %s") % content_view_id if cv_name.nil? | ||
| env_name = ::Katello::KTEnvironment.find_by(:id => lifecycle_environment_id)&.name |
There was a problem hiding this comment.
@jeremylenz Shouldn't these use labels and not names?
There was a problem hiding this comment.
Labels now render instead of names:
Could not create the host:
Cannot assign content view environment RHEL_9_LE/Default_Organization_View: The content view has either not been published or has not been promoted to that lifecycle environment.
4943f41 to
62721a5
Compare
qcjames53
approved these changes
Mar 21, 2025
jeremylenz
force-pushed
the
38270-no-first-or-create-silly
branch
from
62721a5 to
55843a2
Compare
|
Rebased |
|
Ruby test failures are related! looking |
jeremylenz
force-pushed
the
38270-no-first-or-create-silly
branch
from
55843a2 to
d762594
Compare
|
updated tests |
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.
What are the changes introduced in this pull request?
Previously Katello would create a
Katello::ContentViewEnvironmentwhen creating a host, if none existed with that combination of cv/lce. The problem is that creating one without the corresponding Pulp content being hosted (e.g. publishing/promoting the content view to the LCE) will break things.Considerations taken when implementing this change?
The UI already guards against this, so I think this fix is mainly for Robottelo.
What are the testing steps for this pull request?
Try
hammer host createwith--lifecycle-environment-idand--content-view-idoptionsWith a valid combination (published/promoted CV), it should work.
With an invalid combination, you should get one of the new error messages (see the code).