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

Added ability to create replica 3 arbiter 1 gluster volumes #48774

Merged
merged 6 commits into from Jul 26, 2018

Conversation

Projects
None yet
2 participants
@jdito
Copy link
Contributor

commented Jul 25, 2018

What does this PR do?

Adds the ability to create replica 3 arbiter 1 gluster volumes

What issues does this PR fix or reference?

#42955

Previous Behavior

Arbiter configuration was not exposed in glusterfs volume creation

New Behavior

Volumes can be created with arbiter (metadata-only replication) bricks

Tests written?

No

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

jdito@sum-dev-03:/data/salt/salt/gluster$ cat cluster-startup.sls 
peer-cluster:
  glusterfs.peered:
    - names:
      - sum-glfiler-01.storage.dy.gl
      - sum-glfiler-02.storage.dy.gl
      - sum-glfiler-03.storage.dy.gl

{% for volume in ['users','public'] %}
{{ volume }}:
  glusterfs.volume_present:
    - bricks:
      - sum-glfiler-01.storage.dy.gl:/gluster/{{ volume }}/brick
      - sum-glfiler-02.storage.dy.gl:/gluster/{{ volume }}/brick
      - sum-glfiler-03.storage.dy.gl:/gluster/{{ volume }}/brick
    - replica: 3
    - arbiter: True
    - start: True
    - force: True

{% endfor %}
[root@sum-glfiler-03 ~]# salt-call state.apply gluster.cluster-startup
local:
----------
          ID: peer-cluster
    Function: glusterfs.peered
        Name: sum-glfiler-01.storage.dy.gl
      Result: True
     Comment: Host sum-glfiler-01.storage.dy.gl already peered
     Started: 15:31:24.035879
    Duration: 64.799 ms
     Changes:   
----------
          ID: peer-cluster
    Function: glusterfs.peered
        Name: sum-glfiler-02.storage.dy.gl
      Result: True
     Comment: Host sum-glfiler-02.storage.dy.gl already peered
     Started: 15:31:24.101094
    Duration: 58.168 ms
     Changes:   
----------
          ID: peer-cluster
    Function: glusterfs.peered
        Name: sum-glfiler-03.storage.dy.gl
      Result: True
     Comment: Peering with localhost is not needed
     Started: 15:31:24.159656
    Duration: 33.415 ms
     Changes:   
----------
          ID: users
    Function: glusterfs.volume_present
      Result: True
     Comment: Volume users is created and is started
     Started: 15:31:24.193509
    Duration: 4356.89 ms
     Changes:   
              ----------
              new:
                  - users
              old:
----------
          ID: public
    Function: glusterfs.volume_present
      Result: True
     Comment: Volume public is created and is started
     Started: 15:31:28.550847
    Duration: 3752.007 ms
     Changes:   
              ----------
              new:
                  - public
                  - users
              old:
                  - users

Summary for local
------------
Succeeded: 5 (changed=2)
Failed:    0
------------
Total states run:     5
Total run time:   8.265 s
[root@sum-glfiler-03 ~]# gluster volume info
 
Volume Name: public
Type: Replicate
Volume ID: 395e9b53-434f-4a07-9f55-5500ba7499ff
Status: Started
Snapshot Count: 0
Number of Bricks: 1 x (2 + 1) = 3
Transport-type: tcp
Bricks:
Brick1: sum-glfiler-01.storage.dy.gl:/gluster/public/brick
Brick2: sum-glfiler-02.storage.dy.gl:/gluster/public/brick
Brick3: sum-glfiler-03.storage.dy.gl:/gluster/public/brick (arbiter)
Options Reconfigured:
transport.address-family: inet
nfs.disable: on
performance.client-io-threads: off
 
Volume Name: users
Type: Replicate
Volume ID: 787af828-f09b-47fb-b7b9-88486899438d
Status: Started
Snapshot Count: 0
Number of Bricks: 1 x (2 + 1) = 3
Transport-type: tcp
Bricks:
Brick1: sum-glfiler-01.storage.dy.gl:/gluster/users/brick
Brick2: sum-glfiler-02.storage.dy.gl:/gluster/users/brick
Brick3: sum-glfiler-03.storage.dy.gl:/gluster/users/brick (arbiter)
Options Reconfigured:
transport.address-family: inet
nfs.disable: on
performance.client-io-threads: off

Joe DiTommasso and others added some commits Jul 25, 2018

Joe DiTommasso
@rallytime
Copy link
Contributor

left a comment

Thanks for submitting this addition @jdito - I have a couple of small requests.

@@ -217,8 +217,8 @@ def peer(name):
return _gluster(cmd)


def create_volume(name, bricks, stripe=False, replica=False, device_vg=False,
transport='tcp', start=False, force=False):
def create_volume(name, bricks, stripe=False, replica=False, arbiter=False,

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

Can you move the new kwarg to the end of the list? We don't want to create an API change with this new addition.

This comment has been minimized.

Copy link
@jdito

jdito Jul 26, 2018

Author Contributor

Done.

Valid configuration limited to "replica 3 arbiter 1" per \
Gluster documentation. Every third brick in the brick list \
is used as an arbiter brick.

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

Can you add a .. versionadded:: Fluorine tag to these new docs?

This comment has been minimized.

Copy link
@jdito

jdito Jul 26, 2018

Author Contributor

Done-ish? I wasn't 100% sure on the proper syntax for that tag. I added it just above the new documentation I added, but it looks slightly janky. Let me know if it's OK as-is, or if it should be done differently.

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

Oh, you're close. It needs to be below the arg name. Something like this:

arbiter
    words words words

    .. versionadded:: Fluorine

This comment has been minimized.

Copy link
@jdito

jdito Jul 26, 2018

Author Contributor

Ah! That looks much better. I tried putting it under initially, but didn't have a blank line after the "words words words" bit, so it didn't interpret it as a tag. Fixed now, thanks!

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

Sure thing! Thanks for fixing this up so quickly!

def volume_present(name, bricks, stripe=False, replica=False, device_vg=False,
transport='tcp', start=False, force=False):
def volume_present(name, bricks, stripe=False, replica=False, arbiter=False,
device_vg=False, transport='tcp', start=False, force=False):

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

Same comment here about the API change.

This comment has been minimized.

Copy link
@jdito

jdito Jul 26, 2018

Author Contributor

Done.

replica count for volume
arbiter
use every third brick as arbiter (metadata only)

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

This needs a version tag too. :)

This comment has been minimized.

Copy link
@jdito

jdito Jul 26, 2018

Author Contributor

Done.

Joe DiTommasso added some commits Jul 26, 2018

Joe DiTommasso
Joe DiTommasso
Joe DiTommasso
Valid configuration limited to "replica 3 arbiter 1" per \
Gluster documentation. Every third brick in the brick list \
is used as an arbiter brick.

This comment has been minimized.

Copy link
@rallytime

rallytime Jul 26, 2018

Contributor

Sure thing! Thanks for fixing this up so quickly!

@rallytime rallytime merged commit f428d2a into saltstack:develop Jul 26, 2018

5 of 9 checks passed

jenkins/pr/py2-centos-7 running py2-centos-7...
Details
jenkins/pr/py2-ubuntu-1604 running py2-ubuntu-1604...
Details
jenkins/pr/py3-centos-7 running py3-centos-7...
Details
jenkins/pr/py3-ubuntu-1604 running py3-ubuntu-1604...
Details
WIP ready for review
Details
codeclimate All good!
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details

@jdito jdito deleted the jdito:add-gluster-arbiter branch Jul 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.