OCPBUGS-29271: Rebase lvms registry.access.redhat.com lvms4 lvms operator bundle v4.15.0 #2993

jakobmoellerdev · 2024-02-08T21:02:23Z

Which issue(s) this PR addresses:

Update to LVMS 4.15, also solves https://issues.redhat.com/browse/USHIFT-1874 by moving to an embedded lvmd implementation which has feature parity but uses less containers and slightly fewer resources. The lvmd socket is not necessary.

Closes #

openshift-ci-robot · 2024-02-08T21:02:31Z

@jakobmoellerdev: This pull request references Jira Issue OCPBUGS-29271, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

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

In response to this:

Which issue(s) this PR addresses:

Update to LVMS 4.15, also solves https://issues.redhat.com/browse/USHIFT-1874 by moving to an embedded lvmd implementation which has feature parity but uses less containers and slightly fewer resources. The lvmd socket is not necessary.

Closes #

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

jakobmoellerdev · 2024-02-08T21:05:18Z

/test images

jakobmoellerdev · 2024-02-08T21:32:59Z

/test images

dhellmann · 2024-02-08T21:29:07Z

assets/components/lvms/topolvm-node_daemonset.yaml

-            cpu: 1m
-            memory: 16Mi
+            cpu: 2m
+            memory: 35Mi


Those are still small numbers, but they're doubling. Is that absolutely necessary?

I am removing an entire container whose work is now done in this one. Total usage is still down, but we can of course always reduce further if the requests are continously lower than 70%. However the gains introduced by the container unification are 5-15% from my experience

Nice, that sounds like a good improvement overall.

scripts/auto-rebase/lvms_assets.yaml

dhellmann · 2024-02-08T21:33:05Z

scripts/auto-rebase/rebase-lvms.sh

@@ -181,6 +169,10 @@ download_lvms_operator_bundle_manifest(){
        local namespace="openshift-storage"
        extract_lvms_rbac_from_cluster_service_version "${PWD}" "${csv}" "${namespace}"

+        # Push the configMap to the kube-public namespace so that it is available to all users/apps
+        generate_version_config_map "${version}" "lvms-version" "kube-public"\


Do we get one of these per ${ARCHS} now? Are the contents the same?

The contents are identical but you use one staging environment per arch (not sure when or why this was introduced), so if i want to generate files I have to do this per arch.

If you look at release-aarch64 / x86_64, the docker manifests are exactly the same because we build multi-arch containers, so Im not sure why one per arch is necessary

Maybe that's a legacy from when we were using images that were not multi-arch. @pmtk would likely know.

I recall something about osbuild not handling multiarch images correctly while embedding? @ggiguash is that correct?

Im wondering how that fits together. LVMS has multiarch images since forever (4.12 I believe already had them), so im wondering how they wouldnt work with osbuild? Is that even relevant?

Wait, I may have misread things.

Previously the file was created at "${REPOROOT}/assets/components/lvms/topolvm-configmap_lvms-version.yaml" and now it ends up there because you removed "ignore" in lvms-assets.yaml.

I don't think we should make that change - entries without ignore are the ones we obtain from lvms' CSV, this is not the case for the file - we generate it during rebase, in microshift repo.

I discussed with @jakobmoellerdev what's going on here and now I know the story: version configmap was not generated correctly.

What Jakob proposed here is not strictly adhering to the flow we have in rebase.sh (that is: any file created during rebase, is created directly in assets, not in staging and then copied; like kustomizes per $ARCH for OLM).

However, I think proposed solution is more elegant - we don't need to use intermediate file where version gets dumped and then read during manifest update - it's generated during "download" which also extracts RBACs from CSV, then just copied with the power of lvms_assets.yaml and handle_assets.py. LGTM

I agree. It's still reusing the existing flow, but in a less round-about way.

jakobmoellerdev · 2024-02-08T22:10:17Z

@copejon would be great if you could take a look today, ideally want to open up a rebase with the same functionality for 4.15 so that we finally get LVMS into the correct merge window for microshift-4.15

jakobmoellerdev · 2024-02-08T22:21:29Z

/test images

jakobmoellerdev · 2024-02-08T23:22:36Z

/cherry-pick release-4.15

openshift-cherrypick-robot · 2024-02-08T23:22:38Z

@jakobmoellerdev: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jakobmoellerdev · 2024-02-09T10:26:25Z

/test ocp-conformance-rhel-eus-arm

pmtk · 2024-02-09T10:17:50Z

scripts/auto-rebase/rebase-lvms.sh

@@ -149,12 +139,10 @@ download_lvms_operator_bundle_manifest(){

    # Persist the version of the LVMS operator bundle for use in manifest steps
    local version
-    version="$(parse_version "${bundle_manifest}")"
-    dump_version "${version}"


Seems this is not used anymore. Should we remove it?

pmtk · 2024-02-09T10:23:08Z

scripts/auto-rebase/rebase-lvms.sh

@@ -181,6 +169,10 @@ download_lvms_operator_bundle_manifest(){
        local namespace="openshift-storage"
        extract_lvms_rbac_from_cluster_service_version "${PWD}" "${csv}" "${namespace}"

+        # Push the configMap to the kube-public namespace so that it is available to all users/apps
+        generate_version_config_map "${version}" "lvms-version" "kube-public"\


Wait, I may have misread things.

Previously the file was created at "${REPOROOT}/assets/components/lvms/topolvm-configmap_lvms-version.yaml" and now it ends up there because you removed "ignore" in lvms-assets.yaml.

I don't think we should make that change - entries without ignore are the ones we obtain from lvms' CSV, this is not the case for the file - we generate it during rebase, in microshift repo.

jakobmoellerdev · 2024-02-09T10:56:37Z

@pmtk The file ends up here because it is the canonical way to generate files during rebases. Before the change it was actually run twice.

I changed this logic because it simply does not do what its supposed to do and the lvms assets.yaml was generated directly into the assets directory and was not generated in the staging directory that is usually used in the rebase process.

To replicate, simply run the rebase from current main with the command from the last lvms rebase here without adjusting the script.

We do not obtain any of the RBACs from the CSV, I generate them in the script because you have no other way. Generating the config map with a version imo is the same functionality. Generating content based on the pulled image (in this case the tag instead of the CSV file inside the bundle image). The difference between ignore or not is actually just that ignored ones are ignored in the staging folder. They are still present during rebase but get ignored. They dont actuallyget regenerated, they are static and just dont get updated

jakobmoellerdev · 2024-02-09T11:17:51Z

/test ocp-conformance-rhel-eus-arm

jakobmoellerdev · 2024-02-09T12:33:00Z

/test microshift-metal-tests
/test microshift-metal-tests-arm

pacevedom · 2024-02-09T14:06:31Z

/test microshift-metal-tests

jakobmoellerdev · 2024-02-09T14:56:36Z

/test microshift-metal-tests

jakobmoellerdev · 2024-02-12T15:54:51Z

/test microshift-metal-tests
/test microshift-metal-tests-arm

jakobmoellerdev · 2024-02-12T16:09:25Z

/test microshift-metal-tests

jakobmoellerdev · 2024-02-12T17:07:39Z

/test microshift-metal-tests

jakobmoellerdev · 2024-02-13T11:29:03Z

/test microshift-metal-tests
/test microshift-metal-tests-arm

jakobmoellerdev · 2024-02-13T18:35:43Z

@copejon Ready for Dev Test / Confirmation

pmtk · 2024-02-14T09:43:49Z

deployed using clusterbot, lvms-version CM and image refs in topolvm Pods look good
/lgtm

openshift-ci · 2024-02-14T09:46:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakobmoellerdev, pmtk

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~~ [pmtk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2024-02-14T14:34:32Z

@jakobmoellerdev: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2024-02-14T14:37:04Z

@jakobmoellerdev: Jira Issue OCPBUGS-29271: All pull requests linked via external trackers have merged:

openshift/microshift#2993

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

In response to this:

Which issue(s) this PR addresses:

Update to LVMS 4.15, also solves https://issues.redhat.com/browse/USHIFT-1874 by moving to an embedded lvmd implementation which has feature parity but uses less containers and slightly fewer resources. The lvmd socket is not necessary.

Closes #

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-cherrypick-robot · 2024-02-14T14:37:54Z

@jakobmoellerdev: #2993 failed to apply on top of branch "release-4.15":

Applying: chore: fix version config generation map for lvms
Applying: update last_lvms_rebase.sh
Applying: update LVMS manifests
Using index info to reconstruct a base tree...
M	assets/release/release-aarch64.json
M	assets/release/release-x86_64.json
Falling back to patching base and 3-way merge...
Auto-merging assets/release/release-x86_64.json
CONFLICT (content): Merge conflict in assets/release/release-x86_64.json
Auto-merging assets/release/release-aarch64.json
CONFLICT (content): Merge conflict in assets/release/release-aarch64.json
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 update LVMS manifests
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Feb 8, 2024

openshift-ci bot requested review from copejon and jogeo February 8, 2024 21:06

jakobmoellerdev force-pushed the rebase-lvms-registry.access.redhat.com-lvms4-lvms-operator-bundle-v4.15.0 branch from 6b62ceb to 417336a Compare February 8, 2024 21:10

dhellmann reviewed Feb 8, 2024

View reviewed changes

jakobmoellerdev force-pushed the rebase-lvms-registry.access.redhat.com-lvms4-lvms-operator-bundle-v4.15.0 branch from 417336a to 5ac85e1 Compare February 9, 2024 09:38

jakobmoellerdev mentioned this pull request Feb 9, 2024

OCPBUGS-29281: Rebase lvms registry.access.redhat.com lvms4 lvms operator bundle v4.15.0 cherry picked #2997

Closed

pmtk reviewed Feb 9, 2024

View reviewed changes

jakobmoellerdev added 4 commits February 13, 2024 13:02

chore: fix version config generation map for lvms

a6b89ce

update last_lvms_rebase.sh

16a09f1

update LVMS manifests

58a2cc5

chore: adjust to embedded lvmd instead of separate container

456e8db

chore: remove legacy lvms version functions

594b46c

jakobmoellerdev force-pushed the rebase-lvms-registry.access.redhat.com-lvms4-lvms-operator-bundle-v4.15.0 branch from a97a9ff to 594b46c Compare February 13, 2024 13:02

openshift-ci bot assigned pmtk Feb 14, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2024

openshift-merge-bot bot merged commit 00011b0 into openshift:main Feb 14, 2024
10 checks passed

OCPBUGS-29271: Rebase lvms registry.access.redhat.com lvms4 lvms operator bundle v4.15.0 #2993

OCPBUGS-29271: Rebase lvms registry.access.redhat.com lvms4 lvms operator bundle v4.15.0 #2993

Conversation

jakobmoellerdev commented Feb 8, 2024

openshift-ci-robot commented Feb 8, 2024

jakobmoellerdev commented Feb 8, 2024

jakobmoellerdev commented Feb 8, 2024

Choose a reason for hiding this comment

jakobmoellerdev Feb 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmtk Feb 9, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobmoellerdev commented Feb 8, 2024

jakobmoellerdev commented Feb 8, 2024

jakobmoellerdev commented Feb 8, 2024

openshift-cherrypick-robot commented Feb 8, 2024

jakobmoellerdev commented Feb 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobmoellerdev commented Feb 9, 2024 • edited

jakobmoellerdev commented Feb 9, 2024

jakobmoellerdev commented Feb 9, 2024

pacevedom commented Feb 9, 2024

jakobmoellerdev commented Feb 9, 2024

jakobmoellerdev commented Feb 12, 2024

jakobmoellerdev commented Feb 12, 2024

jakobmoellerdev commented Feb 12, 2024

jakobmoellerdev commented Feb 13, 2024

jakobmoellerdev commented Feb 13, 2024

pmtk commented Feb 14, 2024

openshift-ci bot commented Feb 14, 2024

openshift-ci bot commented Feb 14, 2024

openshift-ci-robot commented Feb 14, 2024

openshift-cherrypick-robot commented Feb 14, 2024

jakobmoellerdev Feb 8, 2024 •

edited

pmtk Feb 9, 2024 •

edited

jakobmoellerdev commented Feb 9, 2024 •

edited