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

ceph: reduce the number of provisioner pods in small K8s cluster #6437

Merged
merged 1 commit into from Oct 15, 2020

Conversation

satoru-takeuchi
Copy link
Member

@satoru-takeuchi satoru-takeuchi commented Oct 15, 2020

Description of your changes:

The default number of provisioner pods are 2. When there is only one node, for example, the CI environment, one of these becomes Pending state. Although it's harmless, it's better to reduce this value to 1.

Which issue is resolved by this Pull Request:
Resolves #6294

Checklist:

  • [o] Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • [o] Skip Tests for Docs: Add the flag for skipping the build if this is only a documentation change. See here for the flag.
  • [o] Skip Unrelated Tests: Add a flag to run tests for a specific storage provider. See test options.
  • [o] Reviewed the developer guide on Submitting a Pull Request
  • [o] Documentation has been updated, if necessary.
  • [o] Unit tests have been added, if necessary.
  • [o] Integration tests have been added, if necessary.
  • [o] Pending release notes updated with breaking and/or notable changes, if necessary.
  • [o] Upgrade from previous release is tested and upgrade user guide is updated, if necessary.
  • [o] Code generation (make codegen) has been run to update object specifications, if necessary.

[test ceph]

@mergify mergify bot added the ceph main ceph tag label Oct 15, 2020
@@ -34,6 +34,12 @@ data:
# Supported values from 0 to 5. 0 for general useful logs, 5 for trace level verbosity.
# CSI_LOG_LEVEL: "0"

# Set the number of replicas of rbdplugin-provisioner
# CSI_RBD_PLUGIN_PROVISIONER_REPLICAS: "2"
Copy link
Member

@Madhu-1 Madhu-1 Oct 15, 2020

Choose a reason for hiding this comment

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

can we just check the number of worker nodes in code and set replica to 1?
With this approach, a user can set a higher value and it creates more replicas

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a nice idea.

I have one question. Do you recommend not to the number of replicas larger than 2? If so, I just modify this PR as you suggested. Otherwise, it's also nice to make it configurable and the default value becomes 1 or 2 depending on the number of nodes.

Copy link
Member

Choose a reason for hiding this comment

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

Yes earlier the count was 3. as it consumes more resources and provisioner will always be in HA, what we thought was 2 is good enough

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll fix it.

@satoru-takeuchi satoru-takeuchi force-pushed the ceph-the-number-of-provisioner-pods branch from 1479965 to 433d9f0 Compare October 15, 2020 14:01
@satoru-takeuchi satoru-takeuchi changed the title ceph: make the number of provisioner pods configurable ceph: reduce the number of provisioner pods in small K8s cluster Oct 15, 2020
defaultProvisionerReplicas, err)
}
n := uint8(len(nodes.Items))
if n < defaultProvisionerReplicas {
Copy link
Member

Choose a reason for hiding this comment

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

To make it more obvious that this is really for the single node case, how about this?

if len(nodes.Items) == 1 {
  tp.ProvisionerReplicas = 1
}

if err != nil {
logger.Errorf("failed to get nodes. Defaulting the number of replicas of provisioner pods to %d. %v",
defaultProvisionerReplicas, err)
}
Copy link
Member

Choose a reason for hiding this comment

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

If there was an error, nodes would be nil, so we need to put what's next in an else block, right?

The default number of provisioner pods are 2. When there is only one node,
for example the CI environment, one of these becomes Pending state.
Although it's harmless, it's better to reduce this value to 1.

Closes: rook#6294

Signed-off-by: Satoru Takeuchi <satoru.takeuchi@gmail.com>
@satoru-takeuchi satoru-takeuchi force-pushed the ceph-the-number-of-provisioner-pods branch from 433d9f0 to e22bedb Compare October 15, 2020 18:25
@travisn
Copy link
Member

travisn commented Oct 15, 2020

@satoru-takeuchi The CI issue was intermittent, I'll go ahead and merge so we can get it included in the release today. Thanks!

@travisn travisn merged commit 662bde6 into rook:master Oct 15, 2020
mergify bot added a commit that referenced this pull request Oct 15, 2020
ceph: reduce the number of provisioner pods in small K8s cluster (bp #6437)
@satoru-takeuchi satoru-takeuchi deleted the ceph-the-number-of-provisioner-pods branch October 22, 2020 20:23
@rajivml
Copy link

rajivml commented Nov 14, 2020

@travisn which rook-ceph version has this fix ? am using 1.4.3 and am facing the same problem

@satoru-takeuchi
Copy link
Member Author

@rajivml It's v1.4.6 or later. In addition, v1.5.0 also includes this fix.

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

Successfully merging this pull request may close these issues.

can't set the number of replica for csi-cephfsplugin-provisioner and csi-rbdplugin-provisioner
4 participants