openshift / assisted-installer-agent

Apache License 2.0
24 stars 81 forks source link

ACM-15078: Fixing agents reuse problem in z/VM after reclaim #817

Closed veera-damisetti closed 3 weeks ago

veera-damisetti commented 1 month ago
openshift-ci-robot commented 1 month ago

@veera-damisetti: This pull request references ACM-15078 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set.

In response to [this](https://github.com/openshift/assisted-installer-agent/pull/817): >- Updated bootLoaderConfigTemplateS390x to add ai.ip_cfg_override to take care of IP and nameserver parameters , which is a mandatory parameter in z/VM as mac address is not persistent. >- Updated paramaters which are being passed to zipl , to include parameters from /proc/cmdline which will help in reusing the agents after reclaim. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fassisted-installer-agent). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci[bot] commented 1 month ago

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 33.33333% with 24 lines in your changes missing coverage. Please review.

Project coverage is 59.50%. Comparing base (1c209d5) to head (478b87e). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/commands/actions/reboot_for_reclaim.go 33.33% 24 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/openshift/assisted-installer-agent/pull/817/graphs/tree.svg?width=650&height=150&src=pr&token=ZYXZPU4167&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift)](https://app.codecov.io/gh/openshift/assisted-installer-agent/pull/817?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) ```diff @@ Coverage Diff @@ ## master #817 +/- ## ========================================== - Coverage 59.70% 59.50% -0.21% ========================================== Files 74 74 Lines 3809 3842 +33 ========================================== + Hits 2274 2286 +12 - Misses 1367 1388 +21 Partials 168 168 ``` | [Files with missing lines](https://app.codecov.io/gh/openshift/assisted-installer-agent/pull/817?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) | Coverage Δ | | |---|---|---| | [...rc/commands/actions/download\_boot\_artifacts\_cmd.go](https://app.codecov.io/gh/openshift/assisted-installer-agent/pull/817?src=pr&el=tree&filepath=src%2Fcommands%2Factions%2Fdownload_boot_artifacts_cmd.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift#diff-c3JjL2NvbW1hbmRzL2FjdGlvbnMvZG93bmxvYWRfYm9vdF9hcnRpZmFjdHNfY21kLmdv) | `19.35% <ø> (ø)` | | | [src/commands/actions/reboot\_for\_reclaim.go](https://app.codecov.io/gh/openshift/assisted-installer-agent/pull/817?src=pr&el=tree&filepath=src%2Fcommands%2Factions%2Freboot_for_reclaim.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift#diff-c3JjL2NvbW1hbmRzL2FjdGlvbnMvcmVib290X2Zvcl9yZWNsYWltLmdv) | `30.00% <33.33%> (+7.77%)` | :arrow_up: |
CrystalChun commented 4 weeks ago

Would be great to see how and why these additional parameters are needed specifically for Z in order for the hosts to be reusable! Maybe a doc or blog post would be the route to show everyone?

veera-damisetti commented 4 weeks ago

Sure @CrystalChun , thanks for the review

In order to reuse the hosts, hosts should be booted with proper/desired network and storage configurations.

In case of Z ( z/VM) ,

HCP IBMZ doc for z/VM for cmdline parameter reference: https://docs.redhat.com/en/documentation/red_hat_advanced_cluster_management_for_kubernetes/2.11/html/clusters/cluster_mce_overview#hosted-bare-metal-adding-agents-ibmz-zvm-lpar

RH doc for explaining more details about each param: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/7/html/installation_guide/chap-installer-booting-ipl-s390#chap-installer-booting-ipl-s390

veera-damisetti commented 4 weeks ago

/assign @eifrach

veera-damisetti commented 3 weeks ago

/test edge-e2e-metal-assisted

veera-damisetti commented 3 weeks ago

/test edge-subsystem-test

openshift-ci[bot] commented 3 weeks ago

@veera-damisetti: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
veera-damisetti commented 3 weeks ago

@eifrach , thanks for the review.

I did changes to have a separate function for the logic and added unit tests for the same, and Please have a look.

eifrach commented 3 weeks ago

/lgtm

eifrach commented 3 weeks ago

/approve

openshift-ci[bot] commented 3 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CrystalChun, eifrach, veera-damisetti

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: - ~~[OWNERS](https://github.com/openshift/assisted-installer-agent/blob/master/OWNERS)~~ [eifrach] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
veera-damisetti commented 3 weeks ago

Thanks @CrystalChun @eifrach , for the quick reviews.

openshift-bot commented 3 weeks ago

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-node-agent This PR has been included in build ose-agent-installer-node-agent-container-v4.18.0-202410311509.p0.gd599caa.assembly.stream.el9. All builds following this will include this PR.