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

OCPBUGS-22216: Make lvmo version visible to users via configmap #2789

Conversation

copejon
Copy link
Contributor

@copejon copejon commented Dec 21, 2023

No description provided.

@openshift-ci openshift-ci bot requested review from pliurh and pmtk December 21, 2023 17:05
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 21, 2023
@copejon copejon changed the title [OCPBUGS-22216] make lvmo version visible to users via configmap [WIP][OCPBUGS-22216] make lvmo version visible to users via configmap Dec 21, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2023
@copejon copejon force-pushed the ocpbugs-22216-lvms-version-visibility branch 4 times, most recently from ed3906c to 92fe96b Compare December 21, 2023 20:31
@copejon
Copy link
Contributor Author

copejon commented Dec 21, 2023

/test test-rebase

@copejon copejon changed the title [WIP][OCPBUGS-22216] make lvmo version visible to users via configmap [OCPBUGS-22216] make lvmo version visible to users via configmap Dec 21, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2023

```shell
$ oc get configmap -n openshift-storage lvms-version -o jsonpath='{.data.version}'
v4.14.0-10
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to display LVMS version when running microshift version as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, we don't want the CSI driver more tightly coupled to the core service, any more than we do for other add-ons.

by image tag, with only the major version correlating the major MicroShift version.

The LVMS version is not exposed by LVMS itself. For troublshooting purposes, MicroShift exposes the LVMS version
via a configmap in the `openshift-storage` namespace. To get the LVMS version, run the following command:
Copy link
Contributor

Choose a reason for hiding this comment

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

@copejon
Copy link
Contributor Author

copejon commented Dec 22, 2023

Something has goofed up the lvmd.yaml default config generation. Looking into it

@ggiguash
Copy link
Contributor

/retitle OCPBUGS-22216: Make lvmo version visible to users via configmap

@openshift-ci openshift-ci bot changed the title [OCPBUGS-22216] make lvmo version visible to users via configmap OCPBUGS-22216: Make lvmo version visible to users via configmap Dec 23, 2023
@openshift-ci-robot 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 Dec 23, 2023
@openshift-ci-robot
Copy link

@copejon: This pull request references Jira Issue OCPBUGS-22216, 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)

Requesting review from QA contact:
/cc @jogeo

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

In response to this:

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 openshift-ci bot requested a review from jogeo December 23, 2023 08:28
@dhellmann
Copy link
Contributor

/retest-required

@dhellmann
Copy link
Contributor

I suspect this needs to be rebased to pick up some of the recent fixes for test flakes.

Copy link
Member

@pmtk pmtk left a comment

Choose a reason for hiding this comment

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

two minor formatting things, but looks good overall

scripts/auto-rebase/lvms_assets.yaml Outdated Show resolved Hide resolved
scripts/auto-rebase/rebase-lvms.sh Outdated Show resolved Hide resolved
@copejon copejon force-pushed the ocpbugs-22216-lvms-version-visibility branch from e61661d to 3bbde74 Compare January 8, 2024 20:01
@copejon
Copy link
Contributor Author

copejon commented Jan 10, 2024

/retest

@copejon copejon force-pushed the ocpbugs-22216-lvms-version-visibility branch 3 times, most recently from 330188f to 509d867 Compare January 11, 2024 23:15
@copejon
Copy link
Contributor Author

copejon commented Jan 12, 2024

/retest

@copejon
Copy link
Contributor Author

copejon commented Jan 15, 2024

/test microshift-metal-tests-arm

@copejon copejon force-pushed the ocpbugs-22216-lvms-version-visibility branch from 6782543 to 5fa1ac3 Compare January 15, 2024 16:35
@@ -373,7 +397,7 @@ usage() {
echo "$(basename "$0") to LVMS_RELEASE_IMAGE Performs all the steps to rebase LVMS"
echo "$(basename "$0") download LVMS_RELEASE_IMAGE Downloads the content of a LVMS release image to disk in preparation for rebasing"
echo "$(basename "$0") images Updates LVMS images"
echo "$(basename "$0") manifests Updates LVMS manifests"
echo "$(basename "$0") manifests LVMS_RELEASE_IMAGE Updates LVMS manifests"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in the other rebase script the manifests command relies on the image already being downloaded. Instead of changing the interface here, could we have the 'download' command put the information needed by the 'manifests' command somewhere to be found later when 'manifests' is run?

kind: ConfigMap
metadata:
name: lvms-version
namespace: openshift-storage
Copy link
Contributor

Choose a reason for hiding this comment

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

The microshift-version config map goes into kube-public to ensure special RBAC rules aren't needed to read it. Will an agent using a service account need special permission to read data from this namespace?

Copy link
Contributor Author

@copejon copejon Jan 17, 2024

Choose a reason for hiding this comment

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

AFAICT, new and existing SAs at least have view access to CMs across the cluster. Tested this by creating a namespace and using it's default SA to read the CM in the openshift-storage namespace.

That said, I'll move change it to the kube-public NS, should those permissions be tightened up in the future.

@copejon copejon force-pushed the ocpbugs-22216-lvms-version-visibility branch 4 times, most recently from 35eeb5f to cd48efc Compare January 22, 2024 19:32
write CM to kube-public ns

Signed-off-by: Jon Cope <jcope@redhat.com>

add lvms version cm to openshift-storage ns

Signed-off-by: Jon Cope <jcope@redhat.com>
@copejon copejon force-pushed the ocpbugs-22216-lvms-version-visibility branch from cd48efc to 690d5cb Compare January 22, 2024 20:14
@copejon
Copy link
Contributor Author

copejon commented Jan 22, 2024

Verify job is broken. Blocked until #2915 is merged

@pacevedom
Copy link
Contributor

/test verify

@pmtk
Copy link
Member

pmtk commented Jan 23, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2024
Copy link
Contributor

openshift-ci bot commented Jan 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: copejon, 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:

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

Copy link
Contributor

openshift-ci bot commented Jan 23, 2024

@copejon: 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-merge-bot openshift-merge-bot bot merged commit 6c8bddd into openshift:main Jan 23, 2024
9 checks passed
@openshift-ci-robot
Copy link

@copejon: Jira Issue OCPBUGS-22216: All pull requests linked via external trackers have merged:

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

In response to this:

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.

@copejon
Copy link
Contributor Author

copejon commented Jan 29, 2024

/cherry-pick release-4.15

@openshift-cherrypick-robot

@copejon: new pull request created: #2946

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants