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

Adds the capability of provisioning with private ip for digitalocean #10093

Merged
merged 3 commits into from
Nov 20, 2020
Merged

Adds the capability of provisioning with private ip for digitalocean #10093

merged 3 commits into from
Nov 20, 2020

Conversation

ufukty
Copy link
Contributor

@ufukty ufukty commented Oct 13, 2020

Summary

  • Adds the capability of provisioning with private ip for digitalocean.
  • Creates an argument which named as connect_with_private_ip to make provisioners to use the private ip instead of the public ip.
  • Creates another argument which named as vpc_uuid for enabling the VPC UUID support that is presented by official DO API

Original problem

Packer is losing connection after provisioner creates a firewall rule that drops the ssh connections coming from outside of local network.

Done

  • make generate
  • make lint displayed an error:
    $ make lint PKG_NAME=builder/digitalocean
    # Pinning golangci-lint at v1.23.8 as --new-from-rev seems to work properly; the latest 1.24.0 has caused issues with memory consumption
    ==> Updating linter dependencies...
    golangci/golangci-lint info checking GitHub for tag 'v1.23.8'
    golangci/golangci-lint info found version: 1.23.8 for v1.23.8/darwin/amd64
    golangci/golangci-lint info installed /Users/ufuktan/go/bin/golangci-lint
    golangci-lint run ./builder/digitalocean/...
    builder/digitalocean/config.go:183:11: S1028: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (gosimple)
    				errs, errors.New(fmt.Sprintf("user_data_file not found: %s", c.UserDataFile)))
    				      ^
    builder/digitalocean/config.go:194:41: S1028: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (gosimple)
    			errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("invalid tag: %s", t)))
    			                                     ^
    make: *** [lint] Error 1
    

@ufukty ufukty requested a review from a team as a code owner October 13, 2020 16:04
@hashicorp-cla
Copy link

hashicorp-cla commented Oct 13, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Oct 13, 2020

Codecov Report

Merging #10093 (2124d3d) into master (01ed398) will increase coverage by 0.00%.
The diff coverage is 72.72%.

Impacted Files Coverage Δ
builder/digitalocean/step_create_droplet.go 10.52% <0.00%> (-0.15%) ⬇️
builder/digitalocean/step_droplet_info.go 0.00% <0.00%> (ø)
builder/digitalocean/config.go 72.04% <100.00%> (+2.63%) ⬆️
packer/communicator.go 75.53% <0.00%> (-1.07%) ⬇️

Copy link
Contributor

@SwampDragons SwampDragons left a comment

Choose a reason for hiding this comment

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

Hi, this looks good! I'd love if you could add a couple of tests to the template logic, but otherwise I think this is good to merge.

builder/digitalocean/config.go Show resolved Hide resolved
@ufukty
Copy link
Contributor Author

ufukty commented Oct 19, 2020

Thank you. Tests you want have been added.

@ufukty
Copy link
Contributor Author

ufukty commented Nov 5, 2020

circle-ci complaining about changes at some files that are irrelevant with this pr. I re-cloned the repo and re-ran make generate-check. Errors seems gone for me.

https://app.circleci.com/pipelines/github/hashicorp/packer/7625/workflows/503eb522-94ce-4361-a027-8193248aa519/jobs/87592/steps

==> Checking that auto-generated code is not changed...
warning: CRLF will be replaced by LF in builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt.
The file will have its original line endings in your working directory
warning: CRLF will be replaced by LF in test/fixtures/provisioner-ansible/largish-file.txt.
The file will have its original line endings in your working directory
diff --git a/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt b/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt
index 9160d5bc3..39d4deeec 100644
--- a/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt
+++ b/builder/azure/arm/azure_error_response_test.TestAzureErrorNestedShouldFormat.approved.txt
@@ -1,4 +1,4 @@
-ERROR: -> DeploymentFailed : At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details.
-ERROR:   -> BadRequest
-ERROR:   -> InvalidRequestFormat : Cannot parse the request.
-ERROR:     -> InvalidJson : Error converting value "playground" to type 'Microsoft.WindowsAzure.Networking.Nrp.Frontend.Contract.Csm.Public.IpAllocationMethod'. Path 'properties.publicIPAllocationMethod', line 1, position 130.
+ERROR: -> DeploymentFailed : At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details.
+ERROR:   -> BadRequest
+ERROR:   -> InvalidRequestFormat : Cannot parse the request.
+ERROR:     -> InvalidJson : Error converting value "playground" to type 'Microsoft.WindowsAzure.Networking.Nrp.Frontend.Contract.Csm.Public.IpAllocationMethod'. Path 'properties.publicIPAllocationMethod', line 1, position 130.
diff --git a/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt b/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt
index d50bdc052..4b4834c62 100644
--- a/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt
+++ b/builder/azure/arm/azure_error_response_test.TestAzureErrorSimpleShouldFormat.approved.txt
@@ -1 +1 @@
-ERROR: -> ResourceNotFound : The Resource 'Microsoft.Compute/images/PackerUbuntuImage' under resource group 'packer-test00' was not found.
+ERROR: -> ResourceNotFound : The Resource 'Microsoft.Compute/images/PackerUbuntuImage' under resource group 'packer-test00' was not found.
diff --git a/test/fixtures/provisioner-ansible/largish-file.txt b/test/fixtures/provisioner-ansible/largish-file.txt
index 4df7b0668..0dec1344a 100644
Binary files a/test/fixtures/provisioner-ansible/largish-file.txt and b/test/fixtures/provisioner-ansible/largish-file.txt differ
Found diffs in go generated code.
You can use the command: `make generate` to reformat code.
make: *** [Makefile:132: generate-check] Error 1

@SwampDragons
Copy link
Contributor

I just reran the test suite and you're right it was some unrelated flake. Thanks for this contribution!

@SwampDragons SwampDragons merged commit 8ec7ee0 into hashicorp:master Nov 20, 2020
@SwampDragons SwampDragons added this to the 1.6.6 milestone Dec 1, 2020
@ghost
Copy link

ghost commented Dec 21, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2020
@ufukty ufukty deleted the digitalocean-connect-with-private-ip branch December 25, 2020 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants