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

MGMT-16687: IBI use separate partition for /var/lib/containers #355

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions ib-cli/README.md
Expand Up @@ -82,9 +82,16 @@ export AUTH_FILE=/path/to/seed-image-pull-secret.json
export PS_FILE=/path/to/release-pull-secret.json
export SSH_PUBLIC_KEY=~/.ssh/id_rsa.pub
export IBI_INSTALLATION_DISK=/dev/sda
# Start of the /var/lib/containers partition. Free space before it will be allocated to system partition
# It can be one of the following:
# - Positive number: partition will start at position 120Gb of the disk and extend to the end of the disk. Example: 120Gb
# - Negative number: partition will be of that precise size. Example: -40Gb
# - If the flag is ommited, no new partition will be created, and a directory /sysroot/containers will be created and used instead
export EXTRA_PARTITION_START=-40G


ib-cli create-iso --installation-disk ${IBI_INSTALLATION_DISK} \
--extra-partition-start ${EXTRA_PARTITION_START} \
--lca-image ${LCA_IMAGE} \
--seed-image ${SEED_IMAGE} \
--seed-version ${SEED_VERSION} \
Expand Down
26 changes: 14 additions & 12 deletions ib-cli/cmd/createiso.go
Expand Up @@ -28,17 +28,18 @@ import (
)

var (
seedImage string
seedVersion string
pullSecretFile string
authFile string
sshPublicKeyFile string
lcaImage string
rhcosLiveIso string
installationDisk string
workDir string
precacheBestEffort bool
precacheDisabled bool
seedImage string
seedVersion string
pullSecretFile string
authFile string
sshPublicKeyFile string
lcaImage string
rhcosLiveIso string
installationDisk string
extraPartitionStart string
workDir string
precacheBestEffort bool
precacheDisabled bool
)

func addFlags(cmd *cobra.Command) {
Expand All @@ -50,6 +51,7 @@ func addFlags(cmd *cobra.Command) {
cmd.Flags().StringVarP(&lcaImage, "lca-image", "l", "quay.io/openshift-kni/lifecycle-agent-operator:4.16.0", "The lifecycle-agent image to use for generating the ISO.")
cmd.Flags().StringVarP(&rhcosLiveIso, "rhcos-live-iso", "r", "https://mirror.openshift.com/pub/openshift-v4/amd64/dependencies/rhcos/latest/rhcos-live.x86_64.iso", "The URL to the rhcos-live-iso for generating the ISO.")
cmd.Flags().StringVarP(&installationDisk, "installation-disk", "i", "", "The disk that will be used for the installation.")
cmd.Flags().StringVarP(&extraPartitionStart, "extra-partition-start", "e", "use_directory", "Start of extra partition used for /var/lib/containers. Partition will expand until the end of the disk. Uses sgdisk notation")
cmd.Flags().StringVarP(&workDir, "dir", "d", "", "The working directory for creating the ISO.")
cmd.Flags().BoolVarP(&precacheBestEffort, "precache-best-effort", "", false, "Set image precache to best effort mode")
cmd.Flags().BoolVarP(&precacheDisabled, "precache-disabled", "", false, "Disable precaching, no image precaching will run")
Expand Down Expand Up @@ -94,7 +96,7 @@ func createIso() error {
}
isoCreator := installationiso.NewInstallationIso(log, op, workDir)
if err = isoCreator.Create(seedImage, seedVersion, authFile, pullSecretFile, sshPublicKeyFile, lcaImage, rhcosLiveIso,
installationDisk, precacheBestEffort, precacheDisabled); err != nil {
installationDisk, extraPartitionStart, precacheBestEffort, precacheDisabled); err != nil {
err = fmt.Errorf("failed to create installation ISO: %w", err)
log.Errorf(err.Error())
return err
Expand Down
3 changes: 2 additions & 1 deletion ib-cli/installationiso/data/ibi-butane.template
Expand Up @@ -36,6 +36,7 @@ systemd:
Environment=SEED_VERSION={{.SeedVersion}}
Environment=LCA_IMAGE={{.LCAImage}}
Environment=INSTALLATION_DISK={{.InstallationDisk}}
Environment=EXTRA_PARTITION_START={{.ExtraPartitionStart}}
{{if .PrecacheDisabled}}
Environment=PRECACHE_DISABLED=true
{{else if .PrecacheBestEffort}}
Expand All @@ -45,4 +46,4 @@ systemd:
RemainAfterExit=yes
ExecStart=/usr/local/bin/install-rhcos-and-restore-seed.sh
[Install]
WantedBy=multi-user.target
WantedBy=multi-user.target
31 changes: 23 additions & 8 deletions ib-cli/installationiso/data/install-rhcos-and-restore-seed.sh
Expand Up @@ -6,27 +6,42 @@ seed_image=${1:-$SEED_IMAGE}
seed_version=${2:-$SEED_VERSION}
installation_disk=${3:-$INSTALLATION_DISK}
lca_image=${4:-$LCA_IMAGE}
extra_partition_start=${5:-$EXTRA_PARTITION_START}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why there's a 5 here? Should be "use directory" no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's the 5th argument from the command line in case you call the install-rhcos-and-restore-seed.sh script 😅

In the systemd unit we feed the data through env variables, but for testing it is useful :)

extra_partition_number=5
extra_partition_label=varlibcontainers
create_extra_partition=true

[[ "$extra_partition_start" == "use_directory" ]] && create_extra_partition="false"

authfile=${AUTH_FILE:-"/var/tmp/backup-secret.json"}
pull_secret=${PULL_SECRET_FILE:-"/var/tmp/pull-secret.json"}

coreos-installer install ${installation_disk}

if [[ "$create_extra_partition" == "true" ]]; then
# Create new partition for /var/lib/containers
sfdisk ${installation_disk} <<< write
sgdisk --new $extra_partition_number:$extra_partition_start --change-name $extra_partition_number:$extra_partition_label ${installation_disk}
mkfs.xfs ${installation_disk}$extra_partition_number
fi


# We need to grow the partition. Coreos-installer leaves a small partition
growpart ${installation_disk} 4
mount ${installation_disk}4 /mnt
mount ${installation_disk}3 /mnt/boot
mount /dev/disk/by-partlabel/root /mnt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we should validate that there enough space left for the os

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This kind of check is not done anywhere while creating the seed
I thought we agreed that user was responsible of partitioning decisions

In any case, what would be a value for "enough space left for the os"
If we know this already, maybe we can just use that number and calculate a default value that gives all the extra space for the new partition, allowing for no user interaction at all in that regards

mount /dev/disk/by-partlabel/boot /mnt/boot
xfs_growfs ${installation_disk}4

# Creating and mounting shared /var/lib/containers
if lsattr -d /mnt/ | cut -d ' ' -f 1 | grep i; then
if [[ "$create_extra_partition" == "true" ]]; then
# Mount extra partition in /var/lib/containers
mount /dev/disk/by-partlabel/varlibcontainers /var/lib/containers
else
# Create and mount directory for /var/lib/containers
chattr -i /mnt/
mkdir -p /mnt/sysroot/containers
mkdir -p /mnt/containers
chattr +i /mnt/
else
mkdir -p /mnt/sysroot/containers
mount -o bind /mnt/containers /var/lib/containers
fi
mount -o bind /mnt/sysroot/containers /var/lib/containers

additional_flags=""
if [ -n "${PRECACHE_DISABLED}" ]; then
Expand Down
46 changes: 24 additions & 22 deletions ib-cli/installationiso/installationiso.go
Expand Up @@ -21,16 +21,17 @@ type InstallationIso struct {
}

type IgnitionData struct {
SeedImage string
SeedVersion string
BackupSecret string
PullSecret string
SshPublicKey string
InstallSeedScript string
LCAImage string
InstallationDisk string
PrecacheBestEffort bool
PrecacheDisabled bool
SeedImage string
SeedVersion string
BackupSecret string
PullSecret string
SshPublicKey string
InstallSeedScript string
LCAImage string
InstallationDisk string
ExtraPartitionStart string
PrecacheBestEffort bool
PrecacheDisabled bool
}

//go:embed data/*
Expand All @@ -56,14 +57,14 @@ const (
)

func (r *InstallationIso) Create(seedImage, seedVersion, authFile, pullSecretFile, sshPublicKeyPath, lcaImage,
rhcosLiveIsoUrl, installationDisk string, precacheBestEffort, precacheDisabled bool) error {
rhcosLiveIsoUrl, installationDisk string, extraPartitionStart string, precacheBestEffort, precacheDisabled bool) error {
r.log.Info("Creating IBI installation ISO")
err := r.validate()
if err != nil {
return err
}
err = r.createIgnitionFile(seedImage, seedVersion, authFile, pullSecretFile, sshPublicKeyPath, lcaImage,
installationDisk, precacheBestEffort, precacheDisabled)
installationDisk, extraPartitionStart, precacheBestEffort, precacheDisabled)
if err != nil {
return err
}
Expand All @@ -87,10 +88,10 @@ func (r *InstallationIso) validate() error {
}

func (r *InstallationIso) createIgnitionFile(seedImage, seedVersion, authFile, pullSecretFile, sshPublicKeyPath, lcaImage,
installationDisk string, precacheBestEffort, precacheDisabled bool) error {
installationDisk string, extraPartitionStart string, precacheBestEffort, precacheDisabled bool) error {
r.log.Info("Generating Ignition Config")
err := r.renderButaneConfig(seedImage, seedVersion, authFile, pullSecretFile, sshPublicKeyPath, lcaImage,
installationDisk, precacheBestEffort, precacheDisabled)
installationDisk, extraPartitionStart, precacheBestEffort, precacheDisabled)
if err != nil {
return err
}
Expand Down Expand Up @@ -154,7 +155,7 @@ func (r *InstallationIso) embedIgnitionToIso() error {
}

func (r *InstallationIso) renderButaneConfig(seedImage, seedVersion, authFile, pullSecretFile, sshPublicKeyPath, lcaImage,
installationDisk string, precacheBestEffort, precacheDisabled bool) error {
installationDisk string, extraPartitionStart string, precacheBestEffort, precacheDisabled bool) error {
r.log.Debug("Generating butane config")
var sshPublicKey []byte
var err error
Expand Down Expand Up @@ -187,13 +188,14 @@ func (r *InstallationIso) renderButaneConfig(seedImage, seedVersion, authFile, p
}

templateData := IgnitionData{SeedImage: seedImage,
SeedVersion: seedVersion,
BackupSecret: backupSecretInButane,
PullSecret: pullSecretInButane,
SshPublicKey: string(sshPublicKey),
InstallSeedScript: seedInstallScriptInButane,
LCAImage: lcaImage,
InstallationDisk: installationDisk,
SeedVersion: seedVersion,
BackupSecret: backupSecretInButane,
PullSecret: pullSecretInButane,
SshPublicKey: string(sshPublicKey),
InstallSeedScript: seedInstallScriptInButane,
LCAImage: lcaImage,
InstallationDisk: installationDisk,
ExtraPartitionStart: extraPartitionStart,
}
if precacheBestEffort {
templateData.PrecacheBestEffort = true
Expand Down
15 changes: 8 additions & 7 deletions ib-cli/installationiso/installationiso_test.go
Expand Up @@ -148,12 +148,13 @@ func TestInstallationIso(t *testing.T) {
},
}
var (
mockController = gomock.NewController(t)
mockOps = ops.NewMockOps(mockController)
seedImage = "seedImage"
seedVersion = "seedVersion"
lcaImage = "lcaImage"
installationDisk = "/dev/sda"
mockController = gomock.NewController(t)
mockOps = ops.NewMockOps(mockController)
seedImage = "seedImage"
seedVersion = "seedVersion"
lcaImage = "lcaImage"
installationDisk = "/dev/sda"
extraPartitionStart = "-40G"
)

for _, tc := range testcases {
Expand Down Expand Up @@ -209,7 +210,7 @@ func TestInstallationIso(t *testing.T) {
}
installationIso := NewInstallationIso(log, mockOps, tmpDir)
err := installationIso.Create(seedImage, seedVersion, authFilePath, psFilePath, sshPublicKeyPath, lcaImage,
rhcosLiveIsoUrl, installationDisk, tc.precacheBestEffort, tc.precacheDisabled)
rhcosLiveIsoUrl, installationDisk, extraPartitionStart, tc.precacheBestEffort, tc.precacheDisabled)
if tc.expectedError == "" {
assert.Equal(t, err, nil)
data, errReading := os.ReadFile(path.Join(tmpDir, butaneConfigFile))
Expand Down