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

Mark nodes with/without certain label as disabled/enabled by default #583

Closed
paalkr opened this issue May 31, 2019 · 20 comments
Closed

Mark nodes with/without certain label as disabled/enabled by default #583

paalkr opened this issue May 31, 2019 · 20 comments
Assignees
Labels
component/longhorn-manager Longhorn manager (control plane) kind/feature Feature request, new feature
Milestone

Comments

@paalkr
Copy link

paalkr commented May 31, 2019

Ref: #574

Only some of the nodes in my cluster has disks attached that is intended for Longhorn. So I would like to store Longhorn data only on nodes that has a specific label. All other nodes should by default be marked as disabled.

image

@paalkr paalkr changed the title Mark nodes with/without certain label as disabled by default Mark nodes with/without certain label as disabled/enabled by default May 31, 2019
@yasker yasker added component/longhorn-manager Longhorn manager (control plane) kind/feature Feature request, new feature labels May 31, 2019
@yasker yasker added this to the v0.6.0 milestone May 31, 2019
@yasker yasker modified the milestones: v0.6.0, v0.7.0 Jun 10, 2019
@yasker yasker modified the milestones: v0.7.0, v0.6.0 Jul 8, 2019
@yasker yasker modified the milestones: v0.6.0, v0.7.0 Jul 8, 2019
@yasker
Copy link
Member

yasker commented Jul 8, 2019

@ttpcodes

Create Default Disk setting can be enhanced to address this. We can Create Default Disk On Nodes With Annotation only when a certain annotation has been found in the new node. If the setting is empty, then we will create a default disk for every new node. Otherwise, we will check for the existence of the annotation. We can suggest using longhorn: storage-node as the annotation. It still only applies to the new nodes.

@yasker yasker modified the milestones: v0.7.0, v0.6.0 Jul 8, 2019
@ttpcodes
Copy link
Contributor

ttpcodes commented Jul 8, 2019

Per discussion with @yasker, it seems like Create Default Disk should still be a boolean setting, but modified so that instead of switching between enabling and disabling default Disk creation, it should switch between creation on all Nodes versus creation on Nodes that are labeled with a certain label that we will decide on.

@yasker
Copy link
Member

yasker commented Jul 8, 2019

@ttpcodes We want to use annotations in this case instead of labels since we didn't expect users to frequently search for the annotation. And it's a one-time operation, happens only when adding the node.

@paalkr
Copy link
Author

paalkr commented Jul 8, 2019

I would strongly encourage to follow @ttpcodes suggestion to use node labels and not node annotations to determine if a node is suitable for a longhorn disk or not. The common way in Kubernetes is to specify node capabilities using labels, not annotations. Annotations is usually describing arbitrary node metadata, not node capabilities.

@yasker
Copy link
Member

yasker commented Jul 8, 2019

@paalkr Fair enough. Labels are more likely used for classification in Kubernetes.

Though what if the user removed a label from the node? I don't think we can remove the default disk automatically due to there can still be replica data on it.

@paalkr
Copy link
Author

paalkr commented Jul 8, 2019

Will that be any different then if a user removed an annotation? Never the less,
my intention when creating this ticket was to address the one shot process that executes once when a new node is added to a cluster. I did not have in mind that changing a node label (or annotation for that matter) at a later time would have any impact on the node status in Longhorn.

@yasker
Copy link
Member

yasker commented Jul 8, 2019

@paalkr Yeah, I was talking about another thing, not related to use label or annotation. We can use the label.

How about this:

Setting option: Create default disk only with labeled node.

  1. If it's false, we will create a default disk specified by Default Data Path and enable scheduling on all new nodes.

  2. if it's true, we will create a default disk and enable scheduling on the node only when:

    1. the node is labeled longhorn: storage-node, and
    2. the node has no disk available
      Other nodes will have no default disk created and scheduling disabled by default when added.

@paalkr
Copy link
Author

paalkr commented Jul 8, 2019

Sounds reasonable. I assume that the ability to define a custom default data path (#582) will apply in both scenarios?

@yasker
Copy link
Member

yasker commented Jul 8, 2019

@paalkr Yeah, it will work with #582

@paalkr
Copy link
Author

paalkr commented Jul 9, 2019

So how are these settings specified during longhorn system deployment? In a configmap?

@yasker
Copy link
Member

yasker commented Jul 9, 2019

@paalkr That's a good question. We didn't support set up Longhorn settings before the deployment, but it can be done. I've filed #623 for this.

@paalkr
Copy link
Author

paalkr commented Jul 9, 2019

Excellent, thanks! It makes perfect sense to be able to define settings related to default data path (#582) and node labels to automatically enable/disable scheduling, during the initial deployment.

@meldafrawi
Copy link
Contributor

meldafrawi commented Jul 11, 2019

Validation FAILED.

Steps to test:

  • Create a k8s cluster (1 controller, 1 worker)
  • Install Longhorn
  • On settings page, Check Create Default Disk on Labeled Nodes = true, Click Save
  • Add two more workers to k8s cluster, one of them labeled with node.longhorn.io/role: storage

I expected that after the nodes gets added to the cluster, only the node labeled will be Schedulable on Longhorn UI, but both of them are Disabled.

@yasker
Copy link
Member

yasker commented Jul 12, 2019

@ttpcodes ^^ I think we haven't updated the label in the setting usage yet.

@yasker
Copy link
Member

yasker commented Jul 12, 2019

@meldafrawi Fix merged.

@meldafrawi
Copy link
Contributor

Validation FAILED.

@yasker @ttpcodes ^

@yasker
Copy link
Member

yasker commented Jul 12, 2019

@meldafrawi what's your reproduce step?

@meldafrawi
Copy link
Contributor

Steps to test:

  • Create a k8s cluster (1 controller, 1 worker)
  • Install Longhorn
  • On settings page, Check Create Default Disk on Labeled Nodes = true, Click Save
  • Add two more workers to k8s cluster, one of them labeled with node.longhorn.io/role: storage

@yasker
Copy link
Member

yasker commented Jul 12, 2019

@meldafrawi The instruction for setting should have changed. Take a look at the PR.

@meldafrawi
Copy link
Contributor

Validation PASSED.

Steps to test:

  • Create a k8s cluster (1 controller, 1 worker)
  • Install Longhorn
  • On settings page, Check Create Default Disk on Labeled Nodes = true, Click Save
  • Add two more workers to k8s cluster, one of them labeled with node.longhorn.io/create-default-disk: true
    Expected Result: Only the node labeld node.longhorn.io/create-default-disk: true get default disk create and become Schedulable, the other node is Disabled.

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/feature Feature request, new feature
Projects
Status: Closed
Development

No branches or pull requests

4 participants