OCPBUGS-51209: Fixing artifact name issue for vmlinuz in latest rhelVersions #355
Conversation
…mlinuz from kernel.img based on RHCOS version Signed-off-by: DAMISETTI-VEERABHADRARAO <[email protected]>
openshift-ci-robot
added
jira/severity-important
|
@veera-damisetti: This pull request references Jira Issue OCPBUGS-51209, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
the
size/M
Signed-off-by: DAMISETTI-VEERABHADRARAO <[email protected]>
internal/handlers/boot_artifacts.go
Outdated
| @@ -19,23 +20,29 @@ type BootArtifactsHandler struct { | |||
|
|
|||
| var _ http.Handler = &BootArtifactsHandler{} | |||
|
|
|||
| var IsoFileName string | |||
There was a problem hiding this comment.
Don't create or use globals like this.
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Thanks @carbonin for the quick review. Even if we are mirroring ISO and changing the name while providing ISO URL, IsoFileName is always coming with version and RHCOS version. For example, we have ISO URL from mirror, https://mirror.openshift.com/pub/openshift-v4/s390x/dependencies/rhcos/4.18/4.18.1/rhcos-4.18.1-s390x-live.s390x.iso Is this the scenario which you are indicating where user can mirror ISO and change the name ? |
|
/test edge-test |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@veera-damisetti: This pull request references Jira Issue OCPBUGS-51209, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/test edge-test |
Okay, I remember this now. Let me take a closer look when I get time and see if this still makes sense. |
|
Sure @carbonin thanks |
…eter to parseArtifact function and updated unit tests accrodingly Signed-off-by: DAMISETTI-VEERABHADRARAO <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: veera-damisetti The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Okay, so the issue here is still that these values ( In any case I think a simpler solution for you would be to just look for both files in the ISO rather than trying to identify which file is the correct one based on the version. I'm not sure if this is 100% correct as I put it together rather quickly, but maybe something like this instead: diff --git a/internal/handlers/boot_artifacts.go b/internal/handlers/boot_artifacts.go
index 0f09b9c..7e4fc20 100644
--- a/internal/handlers/boot_artifacts.go
+++ b/internal/handlers/boot_artifacts.go
@@ -49,6 +49,15 @@ func parseArtifact(path, arch string) (string, error) {
return artifact, nil
}
+func getArtifactFilePath(artifact string) string {
+ filePath := fmt.Sprintf("/images/pxeboot/%s", artifact)
+ if artifact == "generic.ins" {
+ // s390x only, unlike other artifacts this one is at the root of the ISO
+ filePath = fmt.Sprintf("/%s", artifact)
+ }
+ return filePath
+}
+
func (b *BootArtifactsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet && r.Method != http.MethodHead {
log.Error("Only GET and HEAD methods are supported with this endpoint.")
@@ -70,13 +79,11 @@ func (b *BootArtifactsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
}
isoFileName := b.ImageStore.PathForParams(imagestore.ImageTypeFull, version, arch)
- file_path := fmt.Sprintf("/images/pxeboot/%s", artifact)
- if artifact == "generic.ins" {
- // s390x only, unlike other artifacts this one is at the root of the ISO
- file_path = fmt.Sprintf("/%s", artifact)
+ fileReader, err := isoeditor.GetFileFromISO(isoFileName, getArtifactFilePath(artifact))
+ // retry with the old kernel file name if vmlinuz is not found on s390x
+ if err != nil && arch == "s390x" && artifact == "vmlinuz" {
+ fileReader, err = isoeditor.GetFileFromISO(isoFileName, getArtifactFilePath("kernel.img"))
}
-
- fileReader, err := isoeditor.GetFileFromISO(isoFileName, file_path)
if err != nil {
httpErrorf(w, http.StatusInternalServerError, "Error creating file reader stream: %v", err)
returnMaybe we'd also want to put these artifacts names in constants or something, but I'm just posting this to help you understand my idea for moving this away form relying on parsing out versions in any real way. |
Signed-off-by: DAMISETTI-VEERABHADRARAO <[email protected]>
|
@veera-damisetti: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Fixing artifact name issue for vmlinuz in latest rhelVersions ( starting from 9.6 )
Description
Starting from RHEL9.6 , artifact name for kernel has changed to vmliuz from kernel.img. this is breaking the existing flow in Hosted Control Plane and Assisted Installer.
Updated the logic to change the artifact name based the RHEL Version.
How was this code tested?
Assignees
@AmadeusPodvratnik
cc @v78singh
cc @phani2898
Links
https://issues.redhat.com/browse/OCPBUGS-51209
Checklist
docs, README, etc)