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.

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

Add SAP AutoYast installation profile for sle15 #15888

Merged
merged 1 commit into from Dec 7, 2022

Conversation

badboywj
Copy link
Contributor

@badboywj badboywj commented Nov 10, 2022

Description

Add SAP autoyast installation profile for SLES15SP{2,3,4} for x64_64 and ppc64le

Related ticket

Related MRs

Verification run

Arch Migration: SAP Group Migration Daily Group YaST Maintenance Updates - Development
x86_64 create_hdd_sles4sap_gnome_sle15_sp4 sap_autoyast_create_hdd_15sp4
x86_64 create_hdd_sles4sap_gnome_sle15_sp3 sap_autoyast_create_hdd_15sp3
x86_64 create_hdd_sles4sap_gnome_sle15_sp2 sap_autoyast_create_hdd_15sp2
x86_64 migration_offline_dvd_sles4sap15sp4_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp4_fullcd
x86_64 migration_offline_dvd_sles4sap15sp3_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp3_fullcd
x86_64 migration_offline_dvd_sles4sap15sp2_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp2_fullcd
x86_64 migration_offline_scc_sles4sap15sp4_atmg full_flavor sap_migration_offline_scc_sles4sap15sp4_fullcd
x86_64 migration_offline_scc_sles4sap15sp3_atmg full_flavor sap_migration_offline_scc_sles4sap15sp3_fullcd
x86_64 migration_offline_scc_sles4sap15sp2_atmg full_flavor sap_migration_offline_scc_sles4sap15sp2_fullcd
x86_64 migration_offline_scc_sles4sap15sp4_atmg online_flavor sap_migration_offline_scc_sles4sap15sp4
x86_64 migration_offline_scc_sles4sap15sp3_atmg online_flavor sap_migration_offline_scc_sles4sap15sp3
x86_64 migration_offline_scc_sles4sap15sp2_atmg online_flavor sap_migration_offline_scc_sles4sap15sp2
x86_64 migration_online_zypper_sles4sap15sp4_atmg online_flavor sap_migration_online_zypper_sles4sap15sp4
x86_64 migration_online_zypper_sles4sap15sp3_atmg online_flavor sap_migration_online_zypper_sles4sap15sp3
x86_64 migration_online_zypper_sles4sap15sp2_atmg online_flavor sap_migration_online_zypper_sles4sap15sp2
ppc64le migration_offline_dvd_sles4sap15sp4_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp4_fullcd_pre sap_migration_offline_dvd_sles4sap15sp4_fullcd
ppc64le migration_offline_dvd_sles4sap15sp3_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp3_fullcd_pre sap_migration_offline_dvd_sles4sap15sp3_fullcd
ppc64le migration_offline_dvd_sles4sap15sp2_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp2_fullcd_pre sap_migration_offline_dvd_sles4sap15sp2_fullcd
ppc64le migration_offline_scc_sles4sap15sp4_atmg full_flavor sap_migration_offline_scc_sles4sap15sp4_fullcd_pre sap_migration_offline_scc_sles4sap15sp4_fullcd
ppc64le migration_offline_scc_sles4sap15sp3_atmg full_flavor sap_migration_offline_scc_sles4sap15sp3_fullcd_pre sap_migration_offline_scc_sles4sap15sp3_fullcd
ppc64le migration_offline_scc_sles4sap15sp2_atmg full_flavor sap_migration_offline_scc_sles4sap15sp2_fullcd_pre sap_migration_offline_scc_sles4sap15sp2_fullcd
ppc64le migration_offline_scc_sles4sap15sp4_atmg online_flavor sap_migration_offline_scc_sles4sap15sp4_pre sap_migration_offline_scc_sles4sap15sp4
ppc64le migration_offline_scc_sles4sap15sp3_atmg online_flavor sap_migration_offline_scc_sles4sap15sp3_pre sap_migration_offline_scc_sles4sap15sp3
ppc64le migration_offline_scc_sles4sap15sp2_atmg online_flavor sap_migration_offline_scc_sles4sap15sp2_pre sap_migration_offline_scc_sles4sap15sp2
ppc64le migration_online_zypper_sles4sap15sp4_atmg online_flavor sap_migration_online_zypper_sles4sap15sp4_pre sap_migration_online_zypper_sles4sap15sp4
ppc64le migration_online_zypper_sles4sap15sp3_atmg online_flavor sap_migration_online_zypper_sles4sap15sp3_pre sap_migration_online_zypper_sles4sap15sp3
ppc64le migration_online_zypper_sles4sap15sp2_atmg online_flavor sap_migration_online_zypper_sles4sap15sp2_pre sap_migration_online_zypper_sles4sap15sp2

Note:
all of the above verification run is on my branch issue119698, this is my mistake although there are the same code on these branchs, but it will take much time if restarting all the cases, so I just run these cases again on branch autoyast_for_sap, please ping me if not enough.

Arch Migration: SAP Group Migration Daily Group YaST Maintenance Updates - Development
x86_64 create_hdd_sles4sap_gnome_sle15_sp2 sap_autoyast_create_hdd_15sp2
x86_64 migration_online_zypper_sles4sap15sp4_atmg online_flavor sap_migration_online_zypper_sles4sap15sp4
ppc64le migration_offline_dvd_sles4sap15sp4_atmg full_flavor sap_migration_offline_dvd_sles4sap15sp4_fullcd_pre sap_migration_offline_dvd_sles4sap15sp4_fullcd
ppc64le migration_offline_scc_sles4sap15sp4_atmg online_flavor sap_migration_offline_scc_sles4sap15sp4_pre sap_migration_offline_scc_sles4sap15sp4
ppc64le migration_online_zypper_sles4sap15sp4_atmg online_flavor sap_migration_online_zypper_sles4sap15sp4_pre sap_migration_online_zypper_sles4sap15sp4

@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files

@badboywj badboywj force-pushed the autoyast_for_sap branch 2 times, most recently from d756fae to 065ce1d Compare November 24, 2022 02:55
@coolgw coolgw added the qe-yam label Nov 25, 2022
Copy link
Contributor

@jknphy jknphy left a comment

Choose a reason for hiding this comment

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

I'm comparing existing SAP test suite and the ones from your PR and I can see some differences at first view:
The way to boot the system, the target should be graphical:
https://openqa.suse.de/tests/9987594#step/boot_to_desktop/4
https://openqa.suse.de/tests/10027996#step/boot_to_desktop/4
We need to schedule the test modules at the end saptune netweaver.. safconf etc.
Could you please check that there is an equivalent.
Another point for clarity, could you pick for example the PR as a central place where to keep updated the verifications, and pair them up, AutoYaST with corresponding migration, it would be great for review

schedule/yast/autoyast/autoyast_sap.yaml Outdated Show resolved Hide resolved
schedule/yast/autoyast/autoyast_sap.yaml Outdated Show resolved Hide resolved
@jknphy
Copy link
Contributor

jknphy commented Nov 28, 2022

Could you also please link the MRs in the PR's first comment, and the PR in the MRs in the same way, so we can easily navigate them back and forth for review?

@jknphy
Copy link
Contributor

jknphy commented Nov 30, 2022

Commit and PR title:
s/autoyast/AutoYasT/
s/sap/SAP/

@badboywj badboywj force-pushed the autoyast_for_sap branch 2 times, most recently from 8f26689 to e1df08b Compare December 1, 2022 02:05
@badboywj badboywj changed the title Add sap autoyast installation profile for sle15 Add SAP AutoYast installation profile for sle15 Dec 1, 2022
@lemon-suse
Copy link
Contributor

lemon-suse commented Dec 2, 2022

I'm reviewing the profile, but the VR make me feel confuse, https://openqa.suse.de/tests/10048042#settings

CASEDIR | https://github.com/badboywj/os-autoinst-distri-opensuse.git#issue119698

So could you run VR with this PR? So that I can compare the VR uploaded profile with the profile in the PR. Thanks.

@badboywj
Copy link
Contributor Author

badboywj commented Dec 2, 2022

yes, it is a little confused. there were many commits in branch issue119698 because iterative improvement, so I created this new branch to commit code, yes, I didn't consider the issues that would cause confusion. is there any other method to resolve this issue? it will spent much time if re-running all cases with this branch again.

@lemon-suse
Copy link
Contributor

yes, it is a little confused. there were many commits in branch issue119698 because iterative improvement, so I created this new branch to commit code, yes, I didn't consider the issues that would cause confusion. is there any other method to resolve this issue? it will spent much time if re-running all cases with this branch again.

Please run one job with the PR URL, I think it will be enough for me to review :) If have further requirement, I have contact you then.

@badboywj
Copy link
Contributor Author

badboywj commented Dec 2, 2022

Please run one job with the PR URL, I think it will be enough for me to review :) If have further requirement, I have contact you then.

OK, no problem, I will run two cases, one for x86_64, and one for ppc64le, I will paste them in description once done, and ping you.

@JRivrain
Copy link
Contributor

JRivrain commented Dec 2, 2022

A lot of good work here, but it' difficult to review: we should have the rule of 1 ticket = 1 PR (or more), but not 2 tickets = 1 PR, with only one commit. At least it would have been good to separate in 2 commits.

@badboywj badboywj closed this Dec 4, 2022
@badboywj badboywj reopened this Dec 4, 2022
@badboywj
Copy link
Contributor Author

badboywj commented Dec 5, 2022

A lot of good work here, but it' difficult to review: we should have the rule of 1 ticket = 1 PR (or more), but not 2 tickets = 1 PR, with only one commit. At least it would have been good to separate in 2 commits.

yes, I usually agree with you, and these two tasks are very closely related, if splitting two PRs when reviewing we can't actually guarantee that PR for ticket120088 will pass if PR for ticket119698 passes, so I merged two PRs into one, it is convenience, we can understand the entire workflow from create image/VM to migration done.

@jknphy jknphy merged commit 6cfe56a into os-autoinst:master Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants