Drop --target-version parameter #911
Merged
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
ehelms
force-pushed
the
drop-target-version
branch
from
246121d
to
7e80d7c
Compare
|
Almost LGTM. However I've found out an issue after an upgrade to 6.16 is performed:
it should just say "Nothing to update....." w/o any error |
lpramuk
reviewed
Aug 18, 2024
| @phase = :pre_upgrade_checks | ||
| condition = { :tags => [:upgrade_scenario, phase] } | ||
| matching_scenarios = find_scenarios(condition) | ||
| @version = matching_scenarios.first.target_version |
There was a problem hiding this comment.
if next scenario is not installed (next maintenance is not enabled) and you run "upgrade" then it produces:
undefined method `target_version' for nil:NilClass
Is question mark missing?
Suggested change
| @version = matching_scenarios.first.target_version | |
| @version = matching_scenarios.first.target_version? |
There was a problem hiding this comment.
Suggested change
| @version = matching_scenarios.first.target_version | |
| @version = matching_scenarios.first&.target_version |
Safe operator is &, but I wonder what else that might break
There was a problem hiding this comment.
[root@sat-stream-qa-rhel8 ~]# foreman-maintain upgrade check
Checking for new version of satellite-maintain...
Nothing to update, can't find new version of satellite-maintain.
[root@sat-stream-qa-rhel8 ~]# foreman-maintain upgrade run
Checking for new version of satellite-maintain...
Nothing to update, can't find new version of satellite-maintain.
--------------------------------------------------------------------------------
Upgrade finished.
This doesn't look correct to me. I'd have expected an error "there is nothing to upgrade to" or something like that.
lpramuk
suggested changes
Aug 18, 2024
ehelms
force-pushed
the
drop-target-version
branch
2 times, most recently
from
f34dd75
to
c18b493
Compare
|
Updated and here is the new output: Feel free to suggest any word smithing. |
evgeni
reviewed
Aug 19, 2024
ehelms
force-pushed
the
drop-target-version
branch
from
c18b493
to
11fda4d
Compare
evgeni
approved these changes
Aug 19, 2024
evgeni
reviewed
Aug 20, 2024
This parameter is no longer used by the upgrade runner as it is defined internally by the scenarios.
evgeni
force-pushed
the
drop-target-version
branch
from
11fda4d
to
ab88247
Compare
|
@ehelms I can't re-test since there are other missing changes that you have rebased on |
|
Now it is more broken: I can't upgrade at all |
10 tasks
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.
This parameter is no longer used by the upgrade runner as it is defined internally by the scenarios.