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

RFE: Replace host for same fqdn #108

Merged
merged 4 commits into from May 27, 2020
Merged

Conversation

gobindadas
Copy link
Collaborator

RHBZ: 1832658

@@ -0,0 +1,10 @@
cluster_node:
hosts:
# mention whatever host is provided in gluster_maintenance_cluster_node or in gluster_maintenance_cluster_node_2 either one.

Choose a reason for hiding this comment

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

Improve the comment section. What should be the hostname that should here should be clear. There shouldn't be ambiguity. Something like : "this is the backend network FQDN of one of the active hosts. It can take a value of the hosts in gluster_maintenance_cluster_node

  1. This can be better named as

gluster_maintenance_old_node: <host1-backend-network-FQDN>
gluster_maintenance_new_node: <host1-backend-network-FQDN>
gluster_maintenance_cluster_node: <host2-backend-network-FQDN>
gluster_maintenance_cluster_node_2: <host3-backend-network-FQDN>

Choose a reason for hiding this comment

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

Better names should be good:
gluster_maintenance_old_node:
gluster_maintenance_new_node:
gluster_maintenance_cluster_node:
gluster_maintenance_cluster_node_2:

Copy link

@satheesaran satheesaran left a comment

Choose a reason for hiding this comment

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

I have few comments to improve the comments in the inventory files to replace the host

@satheesaran
Copy link

@gobindadas Please add clean and easily understandable comments in the node_replace_inventory.yml file.

@@ -0,0 +1,10 @@
cluster_node:
hosts:
Copy link
Member

Choose a reason for hiding this comment

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

just adding a comment for cli users:-

make sure whatever host you are providing here is where the playbook should be executed.

lvsize: 200G

# Common configurations
vars:
Copy link
Member

Choose a reason for hiding this comment

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

this part needs to be properly intended, since hc nodes in the top is being implemented as a class type and we are only parsing a single class, and this is not properly intended, its taking 'vars' as another hostname

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is properly intended. I don't think "vars" will take as hosts.

Copy link
Member

Choose a reason for hiding this comment

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

PLAY RECAP ***********************************************************************************************************************************************************************
nixcraft-praji1-rhel8.com : ok=9 changed=3 unreachable=0 failed=1 skipped=15 rescued=0 ignored=0
vars : ok=0 changed=0 unreachable=1 failed=0 skipped=0 rescued=0 ignored=0

Copy link
Member

@pkesavap pkesavap left a comment

Choose a reason for hiding this comment

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

Vars needs to be intended

lvsize: 200G

# Common configurations
vars:
Copy link
Member

Choose a reason for hiding this comment

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

PLAY RECAP ***********************************************************************************************************************************************************************
nixcraft-praji1-rhel8.com : ok=9 changed=3 unreachable=0 failed=1 skipped=15 rescued=0 ignored=0
vars : ok=0 changed=0 unreachable=1 failed=0 skipped=0 rescued=0 ignored=0

@pkesavap
Copy link
Member

also just wanted to know, wouldn't it be better to include two inventory in the same file,
because node_replace_inventory.yml should be executed in the gluster_maintancence host, but the node_prep could does not have any such restriction,
since the node_prep is there for replace host flow, wouldnt it be better to have a good single inventory file for replace host, rather than multiple?

gluster_maintenance_old_node: <gluster_maintenance_old_node>
gluster_maintenance_new_node: <gluster_maintenance_new_node>
gluster_maintenance_cluster_node: <gluster_maintenance_cluster_node>
gluster_maintenance_cluster_node_2: <gluster_maintenance_cluster_node_2>

Choose a reason for hiding this comment

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

Still I feel we need to emphasize on backend network in the name. Something like previous sounds good. 'host-backend-network-FQDN'. That will add more clarity whether to add front-end or back-end.

Copy link

@satheesaran satheesaran left a comment

Choose a reason for hiding this comment

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

I still feel the need for adding hostname along with backend-network-FQDN, and that would clarify the ambiguity

- block:
- name: Gluster host preparation
include_role:
name: "{{ item }}"

Choose a reason for hiding this comment

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

When I executed the playbook, I got this warning - "The loop variable 'item' is already in use." for each task execution. I have replaced it with separate loop_variable and that solved the problem. Please incorporate the same

Copy link

@satheesaran satheesaran left a comment

Choose a reason for hiding this comment

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

While executing the replace_host playbook, I was seeing this warning "The loop variable 'item' is already in use." for every task execution, I have supplemented 'with_items' with loop_control and loop_variable and that solved the problem

@satheesaran
Copy link

@gobindadas While executing the replace_host playbook, I was seeing this warning "The loop variable 'item' is already in use." for every task execution, I have supplemented 'with_items' with loop_control and loop_variable and that solved the problem


  • hosts: hc_nodes
    remote_user: root
    gather_facts: yes
    tasks:
    • block:
      • name: Gluster host preparation
        include_role:
        name: "{{ line_item }}"
        with_items:
        • gluster.infra
        • gluster.features
          loop_control:
          loop_var: line_item
          tags:
      • preparehost

@pkesavap
Copy link
Member

Vars needs to be intended

Apologies, It is properly intended, issue was with my setup

@gobindadas
Copy link
Collaborator Author

Fixed all comments

Copy link

@satheesaran satheesaran left a comment

Choose a reason for hiding this comment

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

Looks good to me

@gobindadas gobindadas merged commit 4be5edd into gluster:master May 27, 2020
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.

None yet

3 participants