OCPBUGS-37943: FIPS based ISO Boot support for s390x arch

[phani2898]

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose:

• Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.
• Fetches the kargs details such as default kargs, kargs size and offset from coreos/kargs.jso instead of calculating it based on the marker COREOS_KARGS_EMBED_AREA which is not present in s390x ISO boot files

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @zaneb
/cc @andfasano

Links

• This PR works in favour of s390x arch which gets consumed by appendKargs function in a decision making in the installer codebase.
• Check out this PR: OCPBUGS-37943: FIPS based ISO Boot support for s390x arch installer#9892
• Corresponding slack discussion : https://redhat-internal.slack.com/archives/C06A57N4WEM/p1745332383117689

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

[openshift-ci-robot]

@phani2898: An error was encountered searching for bug OCPBUGS-37943 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.

You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 403:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose: Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @
/cc @

Links

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

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-robot]

@phani2898: An error was encountered searching for bug OCPBUGS-37943 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.

You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 403:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose: Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @zaneb
/cc @andfasano

Links

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

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-robot]

@phani2898: An error was encountered searching for bug OCPBUGS-37943 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.

You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 403:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose: Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @zaneb
/cc @andfasano

Links

This PR works in favour of s390x arch which gets consumed by appendKargs function in a decision making in the installer codebase. Check out the below PR for the same.

openshift/installer#9892

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

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-robot]

@phani2898: An error was encountered searching for bug OCPBUGS-37943 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.

You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 403:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose:

• Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.
• Fetches the kargs details such as default kargs, kargs size and offset from coreos/kargs.jso instead of calculating it based on the marker COREOS_KARGS_EMBED_AREA which is not present in s390x ISO boot files

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @zaneb
/cc @andfasano

Links

• This PR works in favour of s390x arch which gets consumed by appendKargs function in a decision making in the installer codebase.
• Check out this PR: OCPBUGS-37943: FIPS based ISO Boot support for s390x arch installer#9892
• Corresponding slack discussion : https://redhat-internal.slack.com/archives/C06A57N4WEM/p1745332383117689

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

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.

[phani2898]

/cc @zaneb @andfasano

[zaneb]

Based on previous discussions, the issues preventing the existing NewKargsReader() API from working with s390 were:

• The kargsEmbedAreaBoundariesFinder() needs to use the Offset/Size fields in coreos/kargs.json to locate the embed area, and only fall back to the regexp search when kargs.json is not present.
• NewKargsReader() itself assumes that the args end with a newline instead of using the End field from kargs.json
• Both NewKargsReader() and NewRHCOSStreamReader() stream the original files from the ISO with their changes overlaid, but when both modify the same file (as in the case of s390) this means that only one set of changes get applied.

I was under the impression that we had agreed that the solution to this was to have an API that combined the functionality of both NewKargsReader() and NewRHCOSStreamReader() so that both lots of changes could be overlaid on a single file. In my opinion this would be superior to creating an entirely new API that doesn't work like any of the existing ones so that consumers have to choose which to call based on platform.

[carbonin]

Agree with @zaneb. Let's make the existing implementation work for changing s390x kargs using the json file as input.

Also this approach wouldn't work for SaaS and I think we want this to work the same for ABI and for clusters created in console.redhat.com.

[phani2898]

Agree with @zaneb. Let's make the existing implementation work for changing s390x kargs using the json file as input.

Also this approach wouldn't work for SaaS and I think we want this to work the same for ABI and for clusters created in console.redhat.com.

okay sure, will change accordingly. I introduced this new API because we may have to change few input parameters in the existing NewKargsReader to persist both kargs and ignition together in the same cdboot.img file

[openshift-ci-robot]

@phani2898: An error was encountered searching for bug OCPBUGS-37943 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.

You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 403:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose:

• Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.
• Fetches the kargs details such as default kargs, kargs size and offset from coreos/kargs.jso instead of calculating it based on the marker COREOS_KARGS_EMBED_AREA which is not present in s390x ISO boot files

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @zaneb
/cc @andfasano

Links

• This PR works in favour of s390x arch which gets consumed by appendKargs function in a decision making in the installer codebase.
• Check out this PR: OCPBUGS-37943: FIPS based ISO Boot support for s390x arch installer#9892
• Corresponding slack discussion : https://redhat-internal.slack.com/archives/C06A57N4WEM/p1745332383117689

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

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-robot]

@phani2898: An error was encountered searching for bug OCPBUGS-37943 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.

You do not have the permission to see the specified issue.: request failed. Please analyze the request body for more details. Status code: 403:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

In response to this:

Description

Summary

This PR introduces enhancements to the kargs.go file, particularly focusing on appending kargs into the boot image itself where ignition is already embedded using the staging ISO boot image files. This is necessary for architecture like s390x where ignition and kargs lies in the same boot image (cdboot.img) unlike x86 where we have different boot files for booth.

It includes a newly added function for embedding kernel arguments into boot images EmbedKargsIntoBootImage, plus unit tests for the same to ensure reliability and correctness.

Key Enhancements

EmbedKargsIntoBootImage Function

Purpose:

• Allows embedding additional kernel arguments (kargs) into boot images where ignition is also present in the same at a different offset.
• Fetches the kargs details such as default kargs, kargs size and offset from coreos/kargs.jso instead of calculating it based on the marker COREOS_KARGS_EMBED_AREA which is not present in s390x ISO boot files

Benefit: Enhances flexibility and control over boot-time configurations—crucial for architectures needing to embed the kernel parameters into ignition boot images (e.g., s390x).

Rationale: This is created as a public function to make sure it can be accessed from the installer repo to have decision making in the appendKargs function to call this function instead NewKargsReader function, which relies on several factors which are not relevant for s390x and also archs having to embed kargs into same boot image as ignition content.

How was this code tested?

Unit Tests for EmbedKargsIntoBootImage

The tests simulate real ISO staging scenarios by creating temporary base and staging directories, then exercising both failure cases and success cases.

Failure Scenarios Covered:

• Missing config file: Fails when /coreos/kargs.json cannot be read from the base ISO path.
• Malformed JSON: Fails when kargs.json contains invalid JSON.
• Empty entries: Fails when no files entries are present in kargs.json.
• Missing staging files: Fails if a listed target file does not exist in the staging directory.
• Field size exceeded (explicit size): Errors if combined default + custom kernel args exceed the configured size field.
• Field size exceeded (inferred size): Errors if size=0 but the available file space (based on length/offset) is insufficient.
• Invalid target file: Errors if the target path exists but is not a writable file (e.g., a directory).

Success Scenario:

• Multiple files with offsets: Verifies that custom kernel arguments are correctly embedded into two different files at different offsets, appended immediately after the default arguments.

Functional tests

• Successfully generated Agent ISO image with fips: true having s390x arch
• Successfully created an SNO cluster with the same ISO image where FIPS is enabled after boot
• Performed the same for fips: false as well

Assignees

/cc @zaneb
/cc @andfasano

Links

• This PR works in favour of s390x arch which gets consumed by appendKargs function in a decision making in the installer codebase.
• Check out this PR: OCPBUGS-37943: FIPS based ISO Boot support for s390x arch installer#9892
• Corresponding slack discussion : https://redhat-internal.slack.com/archives/C06A57N4WEM/p1745332383117689

Checklist

• Title and description added to both, commit and PR
• Relevant issues have been associated
• Reviewers have been listed
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit tests (note that code changes require unit tests)

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: phani2898
Once this PR has been reviewed and has the lgtm label, please ask for approval from carbonin. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[phani2898]

Based on previous discussions, the issues preventing the existing NewKargsReader() API from working with s390 were:

• The kargsEmbedAreaBoundariesFinder() needs to use the Offset/Size fields in coreos/kargs.json to locate the embed area, and only fall back to the regexp search when kargs.json is not present.
• NewKargsReader() itself assumes that the args end with a newline instead of using the End field from kargs.json
• Both NewKargsReader() and NewRHCOSStreamReader() stream the original files from the ISO with their changes overlaid, but when both modify the same file (as in the case of s390) this means that only one set of changes get applied.

I was under the impression that we had agreed that the solution to this was to have an API that combined the functionality of both NewKargsReader() and NewRHCOSStreamReader() so that both lots of changes could be overlaid on a single file. In my opinion this would be superior to creating an entirely new API that doesn't work like any of the existing ones so that consumers have to choose which to call based on platform.

yes @zaneb, these are the potential issues why it will not work for s390x but even if this logic fetching kargs from kargs.json is fixed still this will not work because both ignition content and kargs are present in the same image cdboot.img for s390x.

As the current logic reads the cdboot.img from the baseIso file where ignition would not have updated by the time we reach appendingKargs, so we will be overlaying the extra kargs data into the cdboot.img file which is fresh without having the updated ignition content before the kargs and with the readers in the picture, currently NewKargsReader() will return the FileData having the cdboot.img filepath from the baseIso having data with only kargs but not the updated ignition content.

To fix this issue, we would need the cdboot.img in which the ignition content is already updated by the time it reaches NewKargsReader. So we can only find the cdboot.img with the ignition content from a.tmpPath as I understand.

So I had to chose this approach of having new API function EmbedKargsIntoBootImage instead of modifying NewKargsReader itself.

After your's and @carbonin suggestion, I went back and tried to look at the code again to refactor my logic with NewKargsReader & overlays itself rather than my approach but having to read from the baseIso itself to appendKargs is where I am unable to proceed ahead and persist both ignition and kargs into cdboot.img

I can see why we had to read from baseIso at here because we really don't have an ISO packed with already updated changes by that time we are appending kargs

This works fine for x86 because ignition and kargs files are different for it unlike s390x where we have a single image cdboot.img

[openshift-ci]

@phani2898: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-test	ed73def	link	true	/test edge-test
ci/prow/edge-lint	ed73def	link	true	/test edge-lint

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.

[zaneb]

even if this logic fetching kargs from kargs.json is fixed still this will not work because both ignition content and kargs are present in the same image cdboot.img for s390x.

Right, this was captured by the 3rd point:

• Both NewKargsReader() and NewRHCOSStreamReader() stream the original files from the ISO with their changes overlaid, but when both modify the same file (as in the case of s390) this means that only one set of changes get applied.

which was why I suggested:

to have an API that combined the functionality of both NewKargsReader() and NewRHCOSStreamReader() so that both lots of changes could be overlaid on a single file.