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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPROVEMENT] Consolidate the mounts in longhorn-manager and instance-manager #5883

Closed
innobead opened this issue May 8, 2023 · 4 comments
Closed
Assignees
Labels
component/longhorn-manager Longhorn manager (control plane) kind/improvement Request for improvement of existing function priority/0 Must be fixed in this release (managed by PO)
Milestone

Comments

@innobead
Copy link
Member

innobead commented May 8, 2023

Is your improvement request related to a feature? Please describe (馃憤 if you like this request)

As per #5856 (comment), there will be some potential leftover backup store mount points (nfs/cifs) which are built into the Kernel after resetting the backup store.

Currently, the backup store will be mounted in the below two places:

  • longhorn-manager: backup target controller to update backup status from the store
  • replica process: backup & restore

How to detach unused mount points when updating the backup target should be consistent for different cases as mentioned in #5856 (comment), the solution is like longhorn/longhorn-manager#1848

The current situation is if the nfs backup store is reset, there will be a leftover. Before v1.4, A nfs mount is a hard mount for the nfs backup store, so it would cause high CPU usage if there are some leftover mount points. After that, there will be leftover mount points but will not cause critical resource consumption until this issue is fixed.

Describe the solution you'd like

As above

Additional context

After discussed with @derekbit
We agree the scope of this issue is to see if we can move the backup related operation like list, remove from longhorn-manager to longhorn-instance-manager.

  1. We can get rid of using binary to do such operation in longhorn-manager
  2. We only need to take care of mount point in longhorn-instance-manager

cc @ChanYiLin @derekbit @longhorn/qa

@innobead innobead added component/longhorn-manager Longhorn manager (control plane) priority/0 Must be fixed in this release (managed by PO) kind/improvement Request for improvement of existing function labels May 8, 2023
@innobead innobead added this to the v1.6.0 milestone May 8, 2023
@innobead
Copy link
Member Author

innobead commented May 8, 2023

@ChanYiLin go discuss with @derekbit when working on this in 1.6.

@ChanYiLin
Copy link
Contributor

ChanYiLin commented Oct 31, 2023

Updated the conclusion of the discussion in the ticket.

@ChanYiLin ChanYiLin changed the title [IMPROVEMENT] Consolidate the mounts in longhorn-manager and instance-manager [IMPROVEMENT] Migrate Backup related operation to instance-manager Nov 1, 2023
@ChanYiLin
Copy link
Contributor

ChanYiLin commented Nov 1, 2023

Just found the previous discussion about why not put all backup operations to longhorn-instance-manager
longhorn/longhorn-manager#1361 (comment)

So let's keep it the same

@ChanYiLin ChanYiLin changed the title [IMPROVEMENT] Migrate Backup related operation to instance-manager [IMPROVEMENT] Consolidate the mounts in longhorn-manager and instance-manager Nov 1, 2023
@ChanYiLin
Copy link
Contributor

Close the issue since it is duplicated to #5741
The goal is to unmount the mount point in longhorn-instance-manager when the target is unset

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/longhorn-manager Longhorn manager (control plane) kind/improvement Request for improvement of existing function priority/0 Must be fixed in this release (managed by PO)
Projects
None yet
Development

No branches or pull requests

2 participants