You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ladislav Thon <no...@github.com> on 2016/04/12 12:44:44 UTC
[jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
I'm submitting this mainly to start a discussion around https://issues.apache.org/jira/browse/JCLOUDS-1092
I'm by no means insisting on applying this exact fix, in fact I'm suggesting another possible approach in the JIRA. I also didn't run the live tests (yet -- there seems to be quite a lot of them). Again -- I want to start a discussion about how should a proper fix look like.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/257
-- Commit Summary --
* [JCLOUDS-1092] AzureComputeServiceAdapter.resumeNode waits for the node to start
-- File Changes --
M azurecompute/src/main/java/org/jclouds/azurecompute/compute/AzureComputeServiceAdapter.java (15)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/257.patch
https://github.com/jclouds/jclouds-labs/pull/257.diff
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for starting the discussion.
I'd rather go for the second option approach, and try to return the node even when there are ongoing operations on it. Users should assume that if a node exists, calling the `ComputeService.getNodemetadata` won't return null. Otherwise there is not a clear contract for the get node method and no one could safely code if using it. IMO that method returning null is the *real* issue here.
This said, is there any way (and hopefully not a too expensive one) to return something *meaningful* when calling getNode?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-209522611
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ignasi Barrera <no...@github.com>.
It looks like the model currently maps a Deployment instance to a node, and that one can be `null`.
@andreaturli @fmartelli @ilgrosso do you remember the reasoning behind this? Do you have any suggestion on how to properly fix the issue that nodes that have ongoing operations are not returned (neither in the `getNode` nor in the `listNodes` operation)?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-211379303
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ladislav Thon <no...@github.com>.
@andreaturli What's your opinion on the JIRA in question, please?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-209392952
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ignasi Barrera <no...@github.com>.
Closed #257.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#event-636857076
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ignasi Barrera <no...@github.com>.
I've pushed this partial fix to master as [76f90fc9](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/76f90fc9). I'm closing this PR as the proper fix will require a completely different patch, but let's continue the discussion here or in the JIRA issue!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-213128105
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ladislav Thon <no...@github.com>.
@nacx Thanks for chiming in. I'm personally inclined to agree with you. But when I was digging through the Git history, I found that the `getNode` method was changed from a very simple implementation that makes sense on the first sight to the current monstrosity in https://github.com/jclouds/jclouds-labs/commit/d3ff98a0 which added the live tests. So presumably, there was a reason for it. That kinda makes me feel stuck :-)
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-209878884
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ladislav Thon <no...@github.com>.
@nacx That sounds good to me. Shall I add a comment like "this is a temporary workaround for JCLOUDS-1092 and should be removed once the issue is resolved properly"?
I'm very much interested in a proper fix and I'm willing to test any proposed fixes with our code. I can submit another PR with a change to `getNode`, but I really don't know if that change would make sense. Looking at `AzureComputeServiceAdapter.listNodes` -- that method seems OK to me, only the `getNode` method ignores a `deployment` if any member of a `roleInstanceList` has `instanceStatus().isTransient`.
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-212277489
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ignasi Barrera <no...@github.com>.
@Ladicek There is a design issue here regarding the list and get node methods, but I think we could merge this as-is (without marking the JIRA issue as fixed) to minimize its impact while we agree on a proper way to address the "null" thing. WDYT?
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-211978096
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ignasi Barrera <no...@github.com>.
The `listNodes` method [ignores `null` deployments](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute/src/main/java/org/jclouds/azurecompute/compute/AzureComputeServiceAdapter.java#L407) too, so it presents the same issue.
The main problem here is that we map a jclouds `Node` to a `Deployment` instance, and we we have nothing to return when the Azure API returns `null`. I don't really know the best way to fix this. A quick brainstorming makes me think about:
* Changing the model and map a jclouds `Node` to something else that is consistently returned by the API. This, however, would require major changes to the provider.ยก, and I'm not sure there is a better fit than Deployment.
* Maintaining a cache of existing nodes (we already do that with images), and update it on every call to list/get/create node. We could mark a node as "in progress" or whatever if the API returns it as null, so we have always something to return, even if it is the "last known state".
Ideas are welcome!
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-213124813
Re: [jclouds/jclouds-labs] [JCLOUDS-1092]
AzureComputeServiceAdapter.resumeNode waits for the node to start (#257)
Posted by Ladislav Thon <no...@github.com>.
Thanks @nacx.
I think I understand one part of your concern -- Azure \[1\] has a concept of compute service, which can contain multiple virtual machines. I think that this `azurecompute` provider tries really hard to hide that; I think that it even makes an assumption that a compute service always contains a single VM. But there's a lot more concepts in the Azure API that are related (deployments, role instances, ...) and I'm really no expert on this.
\[1\] at least the "classic" deployment model, which this provider is built around; `azurecompute-arm` should be very different, because it's using the "resource manager" deployment model
---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/257#issuecomment-213275436