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

MGMT-19100: Allow install to the existing root filesystem #897

Merged
merged 3 commits into from
Jan 23, 2025
Merged

MGMT-19100: Allow install to the existing root filesystem #897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5913bff
Remove restriction on ISO partition types
carbonin Oct 25, 2024
89cbaa5
add coreos image installer cli arg
carbonin Jan 6, 2025
6a02366
Update assisted-service dependency
carbonin Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ replace (
github.com/metal3-io/baremetal-operator/apis => github.com/openshift/baremetal-operator/apis v0.0.0-20220217140404-6b1ecb71984f
github.com/metal3-io/baremetal-operator/pkg/hardwareutils => github.com/openshift/baremetal-operator/pkg/hardwareutils v0.0.0-20220217140404-6b1ecb71984f
github.com/opencontainers/runc => github.com/opencontainers/runc v1.1.12
github.com/openshift/assisted-service/api => github.com/openshift/assisted-service/api v0.0.0-20241213170211-a5016f125538
github.com/openshift/assisted-service/client => github.com/openshift/assisted-service/client v0.0.0-20241213170211-a5016f125538
github.com/openshift/assisted-service/models => github.com/openshift/assisted-service/models v0.0.0-20241213170211-a5016f125538
github.com/openshift/assisted-service/api => github.com/openshift/assisted-service/api v0.0.0-20250121194432-6bdc1336f21b
github.com/openshift/assisted-service/client => github.com/openshift/assisted-service/client v0.0.0-20250121194432-6bdc1336f21b
github.com/openshift/assisted-service/models => github.com/openshift/assisted-service/models v0.0.0-20250121194432-6bdc1336f21b
sigs.k8s.io/cluster-api-provider-aws => github.com/openshift/cluster-api-provider-aws v0.2.1-0.20201022175424-d30c7a274820
sigs.k8s.io/cluster-api-provider-azure => github.com/openshift/cluster-api-provider-azure v0.1.0-alpha.3.0.20201016155852-4090a6970205
)
12 changes: 6 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,12 +620,12 @@ github.com/openshift/api v0.0.0-20240521212423-414cf30d37be h1:v2sJPAU2bY9quEg9G
github.com/openshift/api v0.0.0-20240521212423-414cf30d37be/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
github.com/openshift/assisted-service v1.0.10-0.20241213170211-a5016f125538 h1:/SkX5GwRyPaj6cIkU/KIOBGXkaJdANQQPGKzSu9jxls=
github.com/openshift/assisted-service v1.0.10-0.20241213170211-a5016f125538/go.mod h1:0Ygx4AsKw93mKw5lwzVGI9fxS70obgvyAuQ2EUu4uLQ=
github.com/openshift/assisted-service/api v0.0.0-20241213170211-a5016f125538 h1:Sxw4JHF7NrRlw/FECcGdOzmSwO+FRcdlaKCz/XPrXIE=
github.com/openshift/assisted-service/api v0.0.0-20241213170211-a5016f125538/go.mod h1:tvE25aC7I07Uz2jUt7gH2E8+a1Rw4W7qtW6/EFb8l64=
github.com/openshift/assisted-service/client v0.0.0-20241213170211-a5016f125538 h1:NPQBK49+y8whcMjlVveIFqA74jSW+DL1Do0fsvuMbbk=
github.com/openshift/assisted-service/client v0.0.0-20241213170211-a5016f125538/go.mod h1:y+GNZgra/iQDYlacSjgscim8ldrSOMtV3jQaSKC3APc=
github.com/openshift/assisted-service/models v0.0.0-20241213170211-a5016f125538 h1:YOjfAf4JJk7dM7/dJ4RvGwPu2tkiNAh1dG3pebvfGXg=
github.com/openshift/assisted-service/models v0.0.0-20241213170211-a5016f125538/go.mod h1:bx9NsPeBkhn7az/qFHo3hW8wFqFj4BmT3g1R7wR9gcw=
github.com/openshift/assisted-service/api v0.0.0-20250121194432-6bdc1336f21b h1:rBnzGYXurwiNU5CWfkS/H889KfqEtOMtbKB9QqnR0nM=
github.com/openshift/assisted-service/api v0.0.0-20250121194432-6bdc1336f21b/go.mod h1:tvE25aC7I07Uz2jUt7gH2E8+a1Rw4W7qtW6/EFb8l64=
github.com/openshift/assisted-service/client v0.0.0-20250121194432-6bdc1336f21b h1:MZGCt4TDClV9+y9GcmZFF5q2Nj9R8hx8ZOkdFgQwZbA=
github.com/openshift/assisted-service/client v0.0.0-20250121194432-6bdc1336f21b/go.mod h1:y+GNZgra/iQDYlacSjgscim8ldrSOMtV3jQaSKC3APc=
github.com/openshift/assisted-service/models v0.0.0-20250121194432-6bdc1336f21b h1:Esst489U8zAunqPsIObczV8ax4CSF5j2vRDVjrrx/hM=
github.com/openshift/assisted-service/models v0.0.0-20250121194432-6bdc1336f21b/go.mod h1:bx9NsPeBkhn7az/qFHo3hW8wFqFj4BmT3g1R7wR9gcw=
github.com/openshift/baremetal-runtimecfg v0.0.0-20220211165258-fe92b9507bec h1:1nC/js8H8s+MBzgu20YDSuVw6IF9Ct5HOR1JLxv572Q=
github.com/openshift/baremetal-runtimecfg v0.0.0-20220211165258-fe92b9507bec/go.mod h1:fIhPgDwf2Vm1tIiZ0cFidPw3dS26AKGSRoiIJnlz84o=
github.com/openshift/custom-resource-status v1.1.3-0.20220503160415-f2fdb4999d87 h1:cHyxR+Y8rAMT6m1jQCaYGRwikqahI0OjjUDhFNf3ySQ=
Expand Down
4 changes: 4 additions & 0 deletions src/commands/actions/install_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ func (a *install) getFullInstallerCommand() string {
installerCmdArgs = append(installerCmdArgs, "--service-ips", strings.Join(a.installParams.ServiceIps, ","))
}

if len(a.installParams.CoreosImage) > 0 {
installerCmdArgs = append(installerCmdArgs, "--coreos-image", a.installParams.CoreosImage)
}

return fmt.Sprintf("%s %s %s", shellescape.QuoteCommand(podmanCmd), swag.StringValue(a.installParams.InstallerImage),
shellescape.QuoteCommand(installerCmdArgs))
}
Expand Down
5 changes: 5 additions & 0 deletions src/commands/actions/install_cmd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,4 +421,9 @@ var _ = Describe("installer test", func() {

})

It("Args adds the CoreosImage from the install command request when set", func() {
installCommandRequest.CoreosImage = "example.com/openshift/coreos:tag"
args := getInstall(installCommandRequest, filesystem, false).Args()
Expect(strings.Join(args, " ")).To(ContainSubstring("--coreos-image example.com/openshift/coreos:tag"))
})
})
9 changes: 0 additions & 9 deletions src/inventory/disks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,6 @@ func (d *disks) checkEligibility(disk *ghw.Disk) (notEligibleReasons []string, i

// Check disk partitions for type, name, and mount points:
for _, partition := range disk.Partitions {
if partition.Type == "iso9660" {
Copy link
Contributor

@paul-maidment paul-maidment Jan 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is there ever a case where the installation media would be ISO9660 ?
Just for clarification, it's accurate to say that isInstallationMedia will only be set if the mountpoint of a partition has the suffix 'iso' is this the only scenario where we would consider the partition to be installation media?

Copy link
Member Author

@carbonin carbonin Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The installation media can be an iso but the partition never is.
I explain this in the commit and PR description, or are you asking about something else?

Just for clarification, it's accurate to say that isInstallationMedia will only be set if the mountpoint of a partition has the suffix 'iso' is this the only scenario where we would consider the partition to be installation media?

Yes, that will be the situation after this commit is merged. And this will successfully eliminate the cd drive device. You can see this in the following example from a machine with the full ISO booted:

[root@localhost core]# mount | grep iso
/dev/sr0 on /run/media/iso type iso9660 (ro,relatime,nojoliet,check=s,map=n,blocksize=2048)

Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure why this change is part of this PR or why this code is making CAPI installs more difficult.

Copy link
Member Author

Choose a reason for hiding this comment

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

From the PR description:

Additionally the validation on eligible disks for iso type partitions is dropped.
This was done because it is not a valid way to detect installation media, but it is a way that config data can be provided to the host during a CAPI install.

I also mention it in the commit message

notEligibleReasons = append(
notEligibleReasons,
"Disk appears to be an ISO installation media (has partition with "+
"type iso9660)",
)
isInstallationMedia = true
}

if strings.HasSuffix(partition.MountPoint, "iso") {
notEligibleReasons = append(
notEligibleReasons,
Expand Down
24 changes: 3 additions & 21 deletions src/inventory/disks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ var _ = Describe("Disks test", func() {
})

It("filters ISO disks / marks them as installation media", func() {
blockInfo, expectedDisks := prepareDisksTest(dependencies, 3)
blockInfo, expectedDisks := prepareDisksTest(dependencies, 2)

blockInfo.Disks[0].Partitions = []*ghw.Partition{
{
Expand All @@ -569,27 +569,9 @@ var _ = Describe("Disks test", func() {
}
expectedDisks[0].IsInstallationMedia = true

blockInfo.Disks[1].Partitions = []*ghw.Partition{
{
Disk: nil,
Name: "partition2",
Label: "partition2-label",
MountPoint: "/some/mount/point",
SizeBytes: 5555,
Type: "iso9660",
IsReadOnly: false,
},
}

expectedDisks[1].InstallationEligibility.Eligible = false
expectedDisks[1].InstallationEligibility.NotEligibleReasons = []string{
"Disk appears to be an ISO installation media (has partition with type iso9660)",
}
expectedDisks[1].IsInstallationMedia = true

// Make sure regular disks don't get marked as installation media
expectedDisks[2].InstallationEligibility.Eligible = true
expectedDisks[2].IsInstallationMedia = false
expectedDisks[1].InstallationEligibility.Eligible = true
expectedDisks[1].IsInstallationMedia = false

mockFetchDisks(dependencies, nil, blockInfo.Disks...)
ret := GetDisks(&config.SubprocessConfig{}, dependencies)
Expand Down
31 changes: 31 additions & 0 deletions ...hub.com/openshift/assisted-service/api/hiveextension/v1beta1/agentclusterinstall_types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions .../github.com/openshift/assisted-service/api/hiveextension/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

64 changes: 64 additions & 0 deletions vendor/github.com/openshift/assisted-service/client/operators/operators_client.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading