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

Fixes major bug which makes jclouds discovery unusable. #6651

Merged

Conversation

kobalski
Copy link

@kobalski kobalski commented Nov 5, 2015

Due to a bug in zone and regions filtering current jclouds implementation discovery is
broken in 3.6-EA. Also added more logs and unit tests, configuration for live
testing is coming after this pr is merged.

}
location = location.getParent();
}
return true;

Choose a reason for hiding this comment

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

if location itself or any of its parents in the graph is not in REGION scope, it returns true. That means to me node is not inside that region but method name is isNodeInsideRegions.
It feels strange...

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it might be feel strange, but a Node in jclouds always has REGION scope, because you cannot define a ComputeResource without binding it to a region or zone. So your case will never happen.

@kobalski kobalski force-pushed the fix/jclouds_zone_region_bugfix branch from c9a704e to 8ca3493 Compare November 5, 2015 11:56
Properties jcloudsProperties = newOverrideProperties();
if (regions != null) {
List<String> regionList = Arrays.asList(regions.split(","));
for (String region : regionList) {
regionsSet.add(region);
Copy link
Contributor

Choose a reason for hiding this comment

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

so now we are supporting multi regions and zones. (with comma separated) right?

Copy link
Author

Choose a reason for hiding this comment

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

Correct.

@kobalski kobalski force-pushed the fix/jclouds_zone_region_bugfix branch from 8ca3493 to 6b3656d Compare November 5, 2015 15:05
@kobalski
Copy link
Author

kobalski commented Nov 6, 2015

Thanks for your comments. Will merge if no more comments are present today.

@kobalski
Copy link
Author

kobalski commented Nov 6, 2015

verify

3 similar comments
@mesutcelik
Copy link

verify

@kobalski
Copy link
Author

kobalski commented Nov 6, 2015

verify

@mesutcelik
Copy link

verify

zone and regions filtering current jclouds implementation discovery is
broken. Also added more logs and unit tests, configuration for live
testing is coming after this pr is merged.
@kobalski kobalski force-pushed the fix/jclouds_zone_region_bugfix branch from 6b3656d to 4dc3110 Compare November 7, 2015 17:46
mesutcelik pushed a commit that referenced this pull request Nov 9, 2015
Fixes major bug which makes jclouds discovery unusable.
@mesutcelik mesutcelik merged commit 75f8da3 into hazelcast:master Nov 9, 2015
@mmedenjak mmedenjak added the Source: Internal PR or issue was opened by an employee label Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants