openshift-kni / lifecycle-agent

Local agent for orchestration of SNO Image Based Upgrade
Apache License 2.0
6 stars 26 forks source link

OCPBUGS-34089: Add container storage mountpoint checks #561

Closed donpenney closed 3 weeks ago

donpenney commented 3 weeks ago

This update adds checks to IBU and seedgen to ensure:

Background / Context

IBU requires container storage setup such that it is shared between stateroots. The configuration of the shared storage is up to the user, but the mountpoint target (eg. /dev/disk/by-partlabel/varlibcontainers) must be the same on both seed and target SNO.

Issue / Requirement / Reason for change

If the seed image is generated on an SNO with a different container storage mountpoint target, the upgraded SNO may fail to boot, as the mountpoint target would not exist.

Solution / Feature Overview

In order to prevent such a configuration mismatch, LCA has been updated to include checks for the container storage setup.

Implementation Details

LCA parses the var-lib-containers.mount service unit to determine the mountpoint target, as defined by the What config variable. This path is added to the seed config information, which is included in the seed image label. This label is checked during IBU Prep to ensure that it matches the configuration on the running cluster.

openshift-ci-robot commented 3 weeks ago

@donpenney: This pull request references Jira Issue OCPBUGS-34089, which is invalid:

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to [this](https://github.com/openshift-kni/lifecycle-agent/pull/561): >This update adds checks to IBU and seedgen to ensure: >- container storage is setup shared, ie. is a mountpoint >- the mountpoint target for the seed image is the same as the mountpoint target on the running cluster > ># Background / Context > >IBU requires container storage setup such that it is shared between stateroots. The configuration of the shared storage is up to the user, but the mountpoint target (eg. `/dev/disk/by-partlabel/varlibcontainers`) must be the same on both seed and target SNO. > ># Issue / Requirement / Reason for change > >If the seed image is generated on an SNO with a different container storage mountpoint target, the upgraded SNO may fail to boot, as the mountpoint target would not exist. > ># Solution / Feature Overview > >In order to prevent such a configuration mismatch, LCA has been updated to include checks for the container storage setup. >- If container storage is not setup shared, ie. a mountpoint: > - A seedgen request will be rejected > - IBU Prep stage will fail >- If the container storage config in the seed image does not align with the running cluster, the IBU Prep stage will fail > ># Implementation Details > >LCA parses the var-lib-containers.mount service unit to determine the mountpoint target, as defined by the `What` config variable. This path is added to the seed config information, which is included in the seed image label. This label is checked during IBU Prep to ensure that it matches the configuration on the running cluster. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift-kni%2Flifecycle-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.
donpenney commented 3 weeks ago

/cc @omertuc @tsorya @jc-rh @browsell

donpenney commented 3 weeks ago

/retest

donpenney commented 3 weeks ago

/retest

browsell commented 3 weeks ago

/lgtm

donpenney commented 3 weeks ago

/retest

browsell commented 3 weeks ago

/lgtm

donpenney commented 3 weeks ago

/retest

browsell commented 3 weeks ago

/approve

openshift-ci[bot] commented 3 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: browsell

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-kni/lifecycle-agent/blob/main/OWNERS)~~ [browsell] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
donpenney commented 3 weeks ago

/override ci/prow/integration

openshift-ci[bot] commented 3 weeks ago

@donpenney: Overrode contexts on behalf of donpenney: ci/prow/integration

In response to [this](https://github.com/openshift-kni/lifecycle-agent/pull/561#issuecomment-2158412227): >/override ci/prow/integration 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.
openshift-ci-robot commented 3 weeks ago

@donpenney: Jira Issue OCPBUGS-34089: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-34089 has been moved to the MODIFIED state.

In response to [this](https://github.com/openshift-kni/lifecycle-agent/pull/561): >This update adds checks to IBU and seedgen to ensure: >- container storage is setup shared, ie. is a mountpoint >- the mountpoint target for the seed image is the same as the mountpoint target on the running cluster > ># Background / Context > >IBU requires container storage setup such that it is shared between stateroots. The configuration of the shared storage is up to the user, but the mountpoint target (eg. `/dev/disk/by-partlabel/varlibcontainers`) must be the same on both seed and target SNO. > ># Issue / Requirement / Reason for change > >If the seed image is generated on an SNO with a different container storage mountpoint target, the upgraded SNO may fail to boot, as the mountpoint target would not exist. > ># Solution / Feature Overview > >In order to prevent such a configuration mismatch, LCA has been updated to include checks for the container storage setup. >- If container storage is not setup shared, ie. a mountpoint: > - A seedgen request will be rejected > - IBU Prep stage will fail >- If the container storage config in the seed image does not align with the running cluster, the IBU Prep stage will fail > ># Implementation Details > >LCA parses the var-lib-containers.mount service unit to determine the mountpoint target, as defined by the `What` config variable. This path is added to the seed config information, which is included in the seed image label. This label is checked during IBU Prep to ensure that it matches the configuration on the running cluster. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift-kni%2Flifecycle-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.