Fixes #36498 - Don't try to unset a host's CV/LCE when inheriting from hostgroup #10595
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
|
Issues: #36498 |
jeremylenz
force-pushed
the
36498-dont-inherit-half-a-cve
branch
from
7edbe46
to
50076fa
Compare
jeremylenz
force-pushed
the
36498-dont-inherit-half-a-cve
branch
from
a7b1183
to
0a5a0e5
Compare
jeremylenz
force-pushed
the
36498-dont-inherit-half-a-cve
branch
from
0a5a0e5
to
c76af7d
Compare
jeremylenz
commented
Jun 12, 2023
| return attributes if facet_attrs.blank? | ||
| cv_id = facet_attrs['content_view_id'] | ||
| lce_id = facet_attrs['lifecycle_environment_id'] | ||
| if initialized && (cv_id.blank? || lce_id.blank?) |
There was a problem hiding this comment.
If both cv_id and lce_id are blank, what then?
I considered adding explicit logic for this. But I think we're okay without it, because the two cases are
- Host with content facet: will already have a valid LCE and CV, and the host will use its existing values
- Host without content facet:
facet_attrswill benil, meaning this code path isn't touched
anyway, I couldn't get it to error in my testing. But let me know if I'm wrong :D
This was referenced Jun 12, 2023
lfu
pushed a commit
to lfu/katello
that referenced
this pull request
Jun 21, 2023
…m hostgroup (Katello#10595) * Fixes #36498 - Don't try to unset a host's CV/LCE when inheriting from hostgroup * Refs #36498 - fix test (cherry picked from commit 08941da)
wbclark
pushed a commit
to wbclark/katello
that referenced
this pull request
Sep 7, 2023
…m hostgroup (Katello#10595) * Fixes #36498 - Don't try to unset a host's CV/LCE when inheriting from hostgroup * Refs #36498 - fix test (cherry picked from commit 08941da)
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?
Redefines Foreman's
apply_inherited_attributesmethod (fromHost::Managed) to address the case where eithercontent_view_idorlifecycle_environment_idare blank. Since they must now be provided together, having a hostgroup where only one of them is set causes problems.Considerations taken when implementing this change?
This whole issue should go away once we make the same multi-CV changes to
Hostgroup::ContentFacetthat have already been made toHost::ContentFacet.What are the testing steps for this pull request?
Create a hostgroup with CV and LCE
Edit the hostgroup and remove the LCE (click the X)
Go to one of your hosts and click Edit
Assign the host to the hostgroup and click Submit
Before patch: an error
content_view_id and lifecycle_environment_id must be provided togetherAfter patch: no errors
Click around other host and hostgroup edit flows and make sure nothing is broken