Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AGENT-972, AGENT-973: Agent minimal ISO support for all platforms #9056

Merged
merged 1 commit into from
Oct 18, 2024
Merged

AGENT-972, AGENT-973: Agent minimal ISO support for all platforms #9056

merged 1 commit into from
Oct 18, 2024

Conversation

bfournie
Copy link
Contributor

@bfournie bfournie commented Sep 26, 2024 •
edited
Loading

Add the ability to enable minimal ISO support for all platform types, not just External. Adds a new flag to the 'agent create image' command.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 26, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 26, 2024 •
edited by openshift-ci bot
Loading

@bfournie: This pull request references AGENT-972 which is a valid jira issue.

This pull request references AGENT-973 which is a valid jira issue.

In response to this:

Add the ability to enable minimal ISO support for all platform types, not just External. Adds a new flag to the 'agent create image' command.

Still WIP as dev-scripts testing needs to be done and integration tests added.

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 openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 26, 2024
@bfournie bfournie force-pushed the agent-minimal-iso branch 2 times, most recently from 305db9a to 5ae1345 Compare September 26, 2024 21:29
zaneb
zaneb reviewed Sep 26, 2024
View reviewed changes
cmd/openshift-install/agent.go Outdated
@@ -129,6 +133,9 @@ func newAgentCreateCmd(ctx context.Context) *cobra.Command {
for _, t := range agentTargets {
t.command.Args = cobra.ExactArgs(0)
t.command.Run = runTargetCmd(ctx, t.assets...)
if t.name == "Agent ISO Image" {
t.command.PersistentFlags().BoolVar(&agentImageMinimalISO, "minimal-iso", false, "generates a minimal ISO image, by default a full ISO will be generated.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to think this should be a flag in the agent-config instead of a CLI option. The reason to use a minimal ISO is that some hardware requires it, so that config should travel along with the hardware configuration, rather than being something the user has to remember to do at runtime.

I originally imagined we would trigger this off the presence of the bootArtifactsBaseURL, but I guess there is a use case for a connected cluster using a minimal ISO where we want to just use the mirror URL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems a fair point to me (let the config travel along witht hw config) for this specific case, it could be worth to be explored

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked to use MinimalISO configuration from agent-config.yaml instead of command line flag.

@bfournie bfournie force-pushed the agent-minimal-iso branch 4 times, most recently from 2571b99 to 24e00da Compare October 1, 2024 11:56
@bfournie bfournie changed the title WIP AGENT-972, AGENT-973: Agent minimal ISO support for all platforms AGENT-972, AGENT-973: Agent minimal ISO support for all platforms Oct 1, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 1, 2024
@bfournie
Copy link
Contributor Author

bfournie commented Oct 1, 2024

/cc @pawanpinjarkar @rwsu

@pawanpinjarkar
Copy link
Contributor

The initial changes look good.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 10, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 10, 2024
@bfournie bfournie force-pushed the agent-minimal-iso branch 2 times, most recently from 2037bd7 to 350a052 Compare October 10, 2024 21:33
@bfournie
Copy link
Contributor Author

/retest

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 16, 2024 •
edited by openshift-ci bot
Loading

@bfournie: This pull request references AGENT-972 which is a valid jira issue.

This pull request references AGENT-973 which is a valid jira issue.

In response to this:

Add the ability to enable minimal ISO support for all platform types, not just External. Adds a new flag to the 'agent create image' command.

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.

@bfournie
WIP AGENT-972, AGENT-973: Agent minimal ISO support for all platforms
3d4734f 
Add the ability to enable minimal ISO support for all platform types,
not just External. Adds a new flag to the 'agent create image' command.

Still WIP as dev-scripts testing needs to be done and integration tests
added.
@andfasano
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 16, 2024
@bfournie
Copy link
Contributor Author

/retest

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 17, 2024

@bfournie: 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/e2e-aws-ovn 5ae1345 link true /test e2e-aws-ovn
ci/prow/okd-e2e-agent-compact-ipv4 24e00da link false /test okd-e2e-agent-compact-ipv4
ci/prow/e2e-agent-compact-ipv4-appliance-diskimage 3d4734f link false /test e2e-agent-compact-ipv4-appliance-diskimage

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.

pawanpinjarkar
pawanpinjarkar approved these changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@pawanpinjarkar pawanpinjarkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pawanpinjarkar

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 18, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit b5438be into openshift:master Oct 18, 2024
25 of 26 checks passed
@bfournie bfournie deleted the agent-minimal-iso branch October 18, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andfasano andfasano andfasano left review comments

@zaneb zaneb zaneb left review comments

@pawanpinjarkar pawanpinjarkar pawanpinjarkar approved these changes

@rwsu rwsu Awaiting requested review from rwsu

Assignees

@andfasano andfasano

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bfournie @openshift-ci-robot @pawanpinjarkar @andfasano @zaneb @openshift-merge-robot