You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2018/05/16 10:13:31 UTC
[jclouds/jclouds] Add default credentials to images created by the
ImageExtension (#1210)
By default, when listing images the ComputeServiceAdapter adds the default credentials for each image. This is not done when images are created by the image extension, and NPEs can appear in code that assumes the default credentials are there, as the field is not nullable.
This change tries to populate the known node credentials for images created form nodes, and falls back to the default strategy to add the default credentials to an image if there are no known credentials.
@danielestevez Can you try this commit and see if it fixes the issues you found with the ImageExtension?
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/1210
-- Commit Summary --
* Add default credentials to images created by the ImageExtension
-- File Changes --
M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/CloudStackComputeService.java (68)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java (48)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeService.java (45)
M compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java (3)
M compute/src/main/java/org/jclouds/compute/extensions/internal/DelegatingImageExtension.java (61)
M compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java (12)
A compute/src/test/java/org/jclouds/compute/extensions/internal/DelegatingImageExtensionTest.java (206)
M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java (6)
M providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/AzureComputeService.java (17)
M providers/gogrid/src/main/java/org/jclouds/gogrid/compute/GoGridComputeService.java (40)
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java (62)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/1210.patch
https://github.com/jclouds/jclouds/pull/1210.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/pull/1210
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
FTR: [JCLOUDS-1421](https://issues.apache.org/jira/browse/JCLOUDS-1421)
--
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/pull/1210#issuecomment-390135482
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
@nacx pushed 1 commit.
3d44352 Add default credentials for all ImageTemplate types
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1210/files/c3022b688c614d5d9cec29a4901f843a48db146a..3d44352db38895533a608af059221816b3fb5ce8
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Daniel Estévez <no...@github.com>.
Some context about what @nacx is talking about
https://stackoverflow.com/questions/50068324/azure-arm-api-returns-locations-with-inconsistent-case
It affects also other of the new regions (not only CanadaEast) and working on it we discovered some unrelated issues like this one with the default credentials. As a separate JIRA from the case insensitive support for this provider.
--
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/pull/1210#issuecomment-389909740
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Andrea Turli <no...@github.com>.
@nacx that sounds like a decent amount of work, I think we should file a jira issue and properly track the change in the next release notes (ideally with a blog post!) - wdyt?
didn't have time yet to review the PR, I'll do asap!
--
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/pull/1210#issuecomment-389904512
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
Closed #1210.
--
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/pull/1210#event-1639491145
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Andrea Turli <no...@github.com>.
andreaturli approved this pull request.
lgtm, thanks @nacx
--
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/pull/1210#pullrequestreview-121530457
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
Merged t [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/b76a594e) ([labs](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/331fd685)) and [2.1.x](http://git-wip-us.apache.org/repos/asf/jclouds/commit/3878e6a6) ([labs](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/fbf1f4bd)).
--
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/pull/1210#issuecomment-391013567
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Daniel Estévez <no...@github.com>.
Tried and fixes the issues with the missing credentials in _testSpawnNodeFromImage_ :+1:
However _testImageIsRemovedFromCacheAfterDeletion_ still fails it _testSpawnNodeFromImage_ is enabled but that's a different 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/pull/1210#issuecomment-389683840
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
Well, yes and no :) @danielestevez is working on fixing jclouds to properly support Azure CanadaEast region. The issue is that the Azure API sometimes returns the region name lowercase, sometimes camelcase, etc, and that messes things up when it comes to filter by location and do several location comparisons jclouds does internally. We still don't know the real scope of this change, but I'm confident it is under control and it won't be a huge deal :)
The image without credentials thing is just isolated to this PR, so I think it is safe to merge (but I'll file a JIRA too).
--
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/pull/1210#issuecomment-389906939
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Daniel Estévez <no...@github.com>.
danielestevez commented on this pull request.
> + // Otherwise fallback to the image default credentials strategy
+ if (template instanceof CloneImageTemplate) {
+ final CloneImageTemplate cloneImageTemplate = (CloneImageTemplate) template;
+ future = Futures.transform(future, new Function<Image, Image>() {
+ @Override
+ public Image apply(Image input) {
+ if (input.getDefaultCredentials() != null) {
+ return input;
+ }
+ Credentials nodeCredentials = credentialStore.get("node#" + cloneImageTemplate.getSourceNodeId());
+ if (nodeCredentials != null) {
+ logger.info(">> Adding node(%s) credentials to image(%s)...", cloneImageTemplate.getSourceNodeId(),
+ cloneImageTemplate.getName());
+ return ImageBuilder.fromImage(input)
+ .defaultCredentials(LoginCredentials.fromCredentials(nodeCredentials)).build();
+ } else {
Remember to add this default credentials also for not CloneImageTemplate
--
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/pull/1210#pullrequestreview-120840206
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Daniel Estévez <no...@github.com>.
Good to know!
Then i guess it's again another problem with the caseInsensitive feature since i always try in one of the problematic regions **CanadaEast**
Will investigate further
--
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/pull/1210#issuecomment-389883645
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> + // Otherwise fallback to the image default credentials strategy
+ if (template instanceof CloneImageTemplate) {
+ final CloneImageTemplate cloneImageTemplate = (CloneImageTemplate) template;
+ future = Futures.transform(future, new Function<Image, Image>() {
+ @Override
+ public Image apply(Image input) {
+ if (input.getDefaultCredentials() != null) {
+ return input;
+ }
+ Credentials nodeCredentials = credentialStore.get("node#" + cloneImageTemplate.getSourceNodeId());
+ if (nodeCredentials != null) {
+ logger.info(">> Adding node(%s) credentials to image(%s)...", cloneImageTemplate.getSourceNodeId(),
+ cloneImageTemplate.getName());
+ return ImageBuilder.fromImage(input)
+ .defaultCredentials(LoginCredentials.fromCredentials(nodeCredentials)).build();
+ } else {
Addressed in the last commit. Thanks for having a look!
--
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/pull/1210#discussion_r188862909
Re: [jclouds/jclouds] Add default credentials to images created by
the ImageExtension (#1210)
Posted by Ignasi Barrera <no...@github.com>.
>However testImageIsRemovedFromCacheAfterDeletion still fails it testSpawnNodeFromImage is enabled but that's a different issue.
I've just run the `AzureComputeImageExtensionLiveTest` in `northeurope` and with this change all pass:
```bash
Running org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest
Configuring TestNG with: TestNG652Configurator
Starting test testCreateImage(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest)
[pool-1-thread-1] Test testCreateImage(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest) succeeded: 423843ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Starting test testImageIsCachedAfterBeingCreated(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest)
Starting test testSpawnNodeFromImage(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest)
[pool-1-thread-2] Test testImageIsCachedAfterBeingCreated(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest) succeeded: 95495ms
Test suite progress: tests succeeded: 2, failed: 0, skipped: 0.
[pool-1-thread-3] Test testSpawnNodeFromImage(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest) succeeded: 442300ms
Test suite progress: tests succeeded: 3, failed: 0, skipped: 0.
Starting test testDeleteImage(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest)
[pool-1-thread-4] Test testDeleteImage(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest) succeeded: 10365ms
Test suite progress: tests succeeded: 4, failed: 0, skipped: 0.
Starting test testImageIsRemovedFromCacheAfterDeletion(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest)
[pool-1-thread-5] Test testImageIsRemovedFromCacheAfterDeletion(org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest) succeeded: 99238ms
Test suite progress: tests succeeded: 5, failed: 0, skipped: 0.
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1,183.506 sec - in org.jclouds.azurecompute.arm.compute.extensions.AzureComputeImageExtensionLiveTest
Results :
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0
```
--
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/pull/1210#issuecomment-389882496