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 flow ExpandCluserWithDetectedPeers #831

Merged
merged 7 commits into from
Feb 26, 2018

Conversation

shtripat
Copy link
Member

This flow would be invoked from tendrl-node-agent on the additionally
detected new peers of the cluster. New nodes would be provisioned with
monitoring etc and imported into the cluster in tendrl system.

tendrl-bug-id: #805
Signed-off-by: Shubhendu shtripat@redhat.com

This flow would be invoked from tendrl-node-agent on the additionally
detected new peers of the cluster. New nodes would be provisioned with
monitoring etc and imported into the cluster in tendrl system.

tendrl-bug-id: Tendrl#805
Signed-off-by: Shubhendu <shtripat@redhat.com>
@shtripat shtripat requested a review from a team as a code owner February 23, 2018 05:07
@shtripat
Copy link
Member Author

}
_cluster.save()

node_ids = self.parameters.get("Node[]", [])
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to detect which nodes are new (check :https://github.com/Tendrl/commons/blob/master/tendrl/commons/flows/import_cluster/__init__.py#L40)

Also, Add a flag "is_managed" to cluster node context, set this from tendrl-gluster-integration service, use this to figure out which nodes are new ("clusternodecontext.is_managed") wouldnt exist for such nodes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought detection code would be separate and after detection, it would invoke this flow. Anyway will try to introduce this is_managed flag with cluster_node_context object and use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I still feel flow should be independent of detection code and as expected it should expect a nodes list for executing the expand of cluster. If that was the case we could just formed and saved jobs while detection itself, why have a separate flow defined for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not talking about detecting the sds or making detected_cluster_id. This flow will be called only after the new nodes have the same TendrlContext (i.e same integration_id). So all you need to do is read list of nodes from here https://github.com/Tendrl/commons/blob/master/tendrl/commons/flows/import_cluster/__init__.py#L38

and then import those nodes for which ClusterNodeContext.is_managed is "no"

enabled: true
inputs:
mandatory:
- "Node[]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Node[] not required

@codecov
Copy link

codecov bot commented Feb 26, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@8660bbf). Click here to learn what that means.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #831   +/-   ##
=========================================
  Coverage          ?   73.32%           
=========================================
  Files             ?       85           
  Lines             ?     3254           
  Branches          ?      408           
=========================================
  Hits              ?     2386           
  Misses            ?      811           
  Partials          ?       57
Impacted Files Coverage Δ
...ows/expand_cluster_with_detected_peers/__init__.py 16.66% <16.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8660bbf...223a239. Read the comment docs.

@r0h4n r0h4n merged commit 089a6b3 into Tendrl:master Feb 26, 2018

loop_count = 0
# Wait for (no of nodes) * 6 minutes for import to complete
wait_count = len(node_ids) * 36
Copy link
Member Author

Choose a reason for hiding this comment

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

@r0h4n node_ids is not the list of nodes with which the cluster is expanded. Rather this is the full list of nodes as few skips might happen at L#70. We should wait for only no of nodes which actually executed import cluster flow here.

Copy link
Contributor

Choose a reason for hiding this comment

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

please send a fix

@r0h4n r0h4n added this to the Milestone 3 (2018) milestone Mar 7, 2018
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.

2 participants