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

refactor(utils): move is mount read only function to common lib #418

Conversation

ChanYiLin
Copy link
Contributor

@ChanYiLin ChanYiLin commented Mar 5, 2024

ref: longhorn/longhorn#7394

Add mount related util function: IsMountPointReadOnly
This is used in share-manager and instance-manager to detect if the mount point becomes readonly.

Note:

  1. Since go-common-libs had some breaking (function parameters not the same) changes in this PR
    So when updating the version, we also need to fix some code to meet the changes in go-common-libs

e.g.

if _, err := nsexec.Execute(nil, "mount", opts, lhtypes.ExecuteDefaultTimeout); err != nil {}
  1. since go-common-libs has been updated,
    • go-spdk-helper is also needed to update
    • longhorn-spdk-engine is also needed to update

cc @derekbit

@ChanYiLin ChanYiLin self-assigned this Mar 5, 2024
@ChanYiLin ChanYiLin requested a review from a team as a code owner March 5, 2024 07:09
@ChanYiLin ChanYiLin force-pushed the LH7394_move_mount_is_readonly_to_common_lib branch from 7a4a8e5 to 2315297 Compare March 5, 2024 07:19
@ChanYiLin ChanYiLin force-pushed the LH7394_move_mount_is_readonly_to_common_lib branch from 2315297 to 71faa72 Compare March 5, 2024 07:27
@ChanYiLin
Copy link
Contributor Author

it seems we should wait until longhorn/longhorn#7578 finished
and longhorn-instance-manager has been updated with the latest lib version
go-spdk-helper, go-common-libs, longhorn-spdk-engine

pkg/proxy/snapshot.go:69:101: not enough arguments in call to c.EngineSnapshotCreate
	have (string, string)
	want (string, string, *api.SnapshotOptions)

cc @derekbit

@derekbit
Copy link
Member

derekbit commented Mar 7, 2024

EngineSnapshotCreate

cc @DamiaSan

Copy link
Member

@innobead innobead left a comment

Choose a reason for hiding this comment

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

LGTM. Resolve the feedback, then we can merge this PR.

go.mod Outdated Show resolved Hide resolved
@ChanYiLin ChanYiLin force-pushed the LH7394_move_mount_is_readonly_to_common_lib branch from 71faa72 to b05bde6 Compare March 7, 2024 06:46
Copy link

mergify bot commented Mar 7, 2024

This pull request is now in conflict. Could you fix it @ChanYiLin? 🙏

@ChanYiLin ChanYiLin force-pushed the LH7394_move_mount_is_readonly_to_common_lib branch from b05bde6 to 768a5d8 Compare March 7, 2024 06:48
ref: longhorn/longhorn 7394

Signed-off-by: Jack Lin <jack.lin@suse.com>
@ChanYiLin ChanYiLin force-pushed the LH7394_move_mount_is_readonly_to_common_lib branch from 768a5d8 to 072ebf8 Compare March 7, 2024 06:52
@innobead innobead merged commit 9ad8b7a into longhorn:master Mar 7, 2024
6 checks passed
@innobead
Copy link
Member

innobead commented Mar 7, 2024

@mergify backport v1.6.x v1.5.x

Copy link

mergify bot commented Mar 7, 2024

backport v1.6.x v1.5.x

✅ Backports have been created

@ChanYiLin
Copy link
Contributor Author

MANUALLY BACKPORT
v16x: #427
v15x: #428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants