Allow specifying of OpenShift stream for install, closes #501 #508
Conversation
smalleni
force-pushed
the
main
branch
3 times, most recently
from
41f0b0f to
f69724a
Compare
|
FYI, output from a successful run with this patch |
|
I believe I have addressed all the comments, please let me know if something slipped. Thanks! |
smalleni
force-pushed
the
main
branch
2 times, most recently
from
85c550c to
39c01b6
Compare
|
Tested in the CPT lab also: |
There was a problem hiding this comment.
I have no idea how common sync-ocp-release is, but I'd feel bad about bringing in a change to break it, and I think it might be easy to fix.
Aside from that and a few minor comments, this is looking really good.
Oh; and making changes during a review by amending the base commit makes it a lot harder to review ... this is GitHub, not Gerrit! I'd prefer to have a series of commits in a PR to allow incremental reviews, with a "squash-and-merge" at the end.
This removes the need to explicitly define the OCP release image and instead requires `ocp_version` and `ocp_build` from the user. OpenShift version could be specific like 4.15.4 or more generic like latest-4.15. The build should denote whether the required OCP version is ga or under development using the keys `ga` or `dev`. This commit also removes the circular dependency on having oc (installed as `oc.latest` to extract the ocp client/installer binaries). Overall, this greatly simplifies the use of jetlag while at the same time making it intelligent to deploy latest ga/nightly versions across minor releases. Closes: redhat-performance#501 Signed-off-by: Sai Sindhur Malleni <[email protected]>
|
Ironically, I think this PR is actually named the opposite of its new feature. (We currently require you to specify a direct or fixed image) I think it should be renamed from |
An interesting point, but really before you specified both the version and the download URL, whereas now you specify only the version (+ build stream), and it supports a more generalized form of build tag. So perhaps neither description really entirely captures the change. Maybe "Construct OpenShift download URL based on version and build stream"??? |
|
After the new PR updates everything looking good in the CPT lab env: |
| @@ -33,7 +48,7 @@ | |||
| when: (cluster_type == "bm" or cluster_type == "rwn") and | |||
| (worker_node_count is undefined or not worker_node_count | int) and | |||
| ansible_play_name != "Create inventory from a lab cloud" and | |||
| ansible_play_name != "Create inventory from ibmcloud hardware" | |||
| ansible_play_name != "Create inventory from ibmcloud hardware" | |||
There was a problem hiding this comment.
Not a real comment here, but I wonder what this whitespace delete then add is about, maybe a github bug?
There was a problem hiding this comment.
I skipped over that since it was whitespace. But I actually played with copying the indent from the raw GitHub file into vscode, and the change is that he's accidentally replaced the final space with a tab. It doesn't affect the visual indent, but it should be reverted!
There was a problem hiding this comment.
I have no clue what's happening here..just used tabs all along in vs code.
There was a problem hiding this comment.
You shouldn't use tab indentation, if only because the visual presentation depends on tab settings of the output device, which is unreliable. In vscode, if you select the whitespace you should see something like ".....->" showing the tab character ... if you delete that character and align the line with spaces (I suspect just one), this minor annoying diff should go away.
akrzos
changed the title
smalleni
force-pushed
the
main
branch
4 times, most recently
from
3b799f2 to
5f0ea23
Compare
Co-authored-by: Alex Krzos <[email protected]> Signed-off-by: Sai Sindhur Malleni <[email protected]>
| @@ -33,7 +48,7 @@ | |||
| when: (cluster_type == "bm" or cluster_type == "rwn") and | |||
| (worker_node_count is undefined or not worker_node_count | int) and | |||
| ansible_play_name != "Create inventory from a lab cloud" and | |||
| ansible_play_name != "Create inventory from ibmcloud hardware" | |||
| ansible_play_name != "Create inventory from ibmcloud hardware" | |||
There was a problem hiding this comment.
You shouldn't use tab indentation, if only because the visual presentation depends on tab settings of the output device, which is unreliable. In vscode, if you select the whitespace you should see something like ".....->" showing the tab character ... if you delete that character and align the line with spaces (I suspect just one), this minor annoying diff should go away.
Remove extra ocp_build var
|
@akrzos @smalleni Well, this is awkward. After rebasing my PR, which is mostly targeted at SNO creation, I tried it and got a failure here in The The problem is that fixing this under the new model is awkward as the openshift version is determined by a subsequent new role; and we can't check We could move this error check later during the SNO post-install configuration (which would be ugly since we wouldn't get the error on startup), or ... well, I'm not coming up with any other great workaround. The simplest hack would be to just say we don't care about 4.9 and 4.10 anymore and delete the check entirely ... 😆 |
This removes the need to explicitly define the OCP release image and instead requires
ocp_versionandocp_buildfrom the user. OpenShift version could be specific like 4.15.4 or more generic like latest-4.15. The build should denote whether the required OCP version is ga or under development using the keysgaordev. This commit also removes the circular dependency on having oc (installed asoc.latestto extract the ocp client/installer binaries). Overall, this greatly simplifies the use of jetlag while at the same time making it intelligent to deploy latest ga/nightly versions across minor releases.Co-authored-by: Alex Krzos [email protected]
Closes: #501