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.

Sign up for GitHub

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

Jump to bottom

feat: use existing IP for server create #144

Merged
merged 19 commits into from
Jan 9, 2024
Merged

feat: use existing IP for server create #144

merged 19 commits into from
Jan 9, 2024

Conversation

sjagoe
Copy link
Contributor

@sjagoe sjagoe commented Jan 8, 2024

This PR adds the ability to specify an existing Primary IP in the Hetzner Cloud project to be used for a packer build.

For short-lived IP addresses, Hetzner sometimes sends abuse notifications to the wrong IP address user.

To avoid this, for most things I run in Hetzner Cloud, I use pre-allocated Primary IPs, set to not auto-delete when the server is removed, so that I have easier tracking of IP address allocation to respond appropriately to Hetzner abuse notifications.

@sjagoe sjagoe requested a review from a team as a code owner January 8, 2024 10:04
apricote
apricote previously requested changes Jan 8, 2024
View reviewed changes
builder/hcloud/step_create_server.go Outdated Show resolved Hide resolved
@sjagoe sjagoe requested a review from apricote January 8, 2024 10:40
@jooola jooola changed the title Allow using existing IP address for server create feat: use existing IP for server create Jan 8, 2024
@jooola
Copy link
Member

jooola commented Jan 8, 2024

Hey @sjagoe
Thanks for the contribution!

Ignore the acceptance tests, they cannot succeed on forks.

Could you regenerate the docs? Using make generate

jooola
jooola requested changes Jan 8, 2024
View reviewed changes
builder/hcloud/step_create_server.go Outdated Show resolved Hide resolved
builder/hcloud/step_create_server.go Outdated Show resolved Hide resolved
@sjagoe sjagoe requested a review from jooola January 8, 2024 16:45
@codecov Codecov
Copy link

codecov bot commented Jan 8, 2024 •
edited
Loading

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (0bc73eb) 27.28% compared to head (4dc0cc8) 29.64%.
Report is 2 commits behind head on main.

Files Patch % Lines
builder/hcloud/step_create_server.go 68.42% 8 Missing and 4 partials ⚠️
builder/hcloud/config.hcl2spec.go 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #144      +/-   ##
==========================================
+ Coverage   27.28%   29.64%   +2.35%     
==========================================
  Files          11       11              
  Lines         612      651      +39     
==========================================
+ Hits          167      193      +26     
- Misses        424      433       +9     
- Partials       21       25       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jooola jooola self-assigned this Jan 8, 2024
@sjagoe sjagoe requested a review from jooola January 9, 2024 09:52
jooola
jooola reviewed Jan 9, 2024
View reviewed changes
builder/hcloud/config.go Outdated Show resolved Hide resolved
@sjagoe sjagoe requested a review from jooola January 9, 2024 12:50
jooola
jooola reviewed Jan 9, 2024
View reviewed changes
docs/builders/hcloud.mdx Outdated Show resolved Hide resolved
jooola
jooola requested changes Jan 9, 2024
View reviewed changes
Copy link
Member

@jooola jooola left a comment

Choose a reason for hiding this comment

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

To be feature complete, the same for ipv6 should also be implemented.

@sjagoe
Copy link
Contributor Author

sjagoe commented Jan 9, 2024

I have implemented IP search by name, and IPv6.

The plugin will still only use IPv4 to connect to the server, though. But there is still a case for assigning a static IPv6 to a server in any case (e.g. allowing the specific IPv6 address through a corporate firewall, for example).

@sjagoe sjagoe requested a review from jooola January 9, 2024 14:29
@sjagoe sjagoe requested a review from jooola January 9, 2024 15:41
jooola
jooola reviewed Jan 9, 2024
View reviewed changes
Copy link
Member

@jooola jooola left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I really appreciate that you are still sticking with us, we are almost done! Please address the last few comments on the tests, and maybe the lint errors will disappear.

Thanks!

builder/hcloud/step_create_server_test.go Outdated Show resolved Hide resolved
builder/hcloud/step_create_server_test.go Outdated Show resolved Hide resolved
builder/hcloud/step_create_server_test.go Show resolved Hide resolved
@sjagoe
Copy link
Contributor Author

sjagoe commented Jan 9, 2024 •
edited
Loading

I really appreciate that you are still sticking with us

Thanks, I appreciate your patience with me. This is the first Go I've written 😄 So building it up in small pieces was helpful to me anyway.

Edit: Lint is happy now 🎉 thanks for the tips.

@sjagoe sjagoe requested a review from jooola January 9, 2024 17:08
jooola
jooola approved these changes Jan 9, 2024
View reviewed changes
Copy link
Member

@jooola jooola left a comment

Choose a reason for hiding this comment

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

Awesome, I ran the integrations tests locally, all green!

Thanks again!

@jooola jooola dismissed apricote’s stale review January 9, 2024 17:15

This is good to go

@jooola jooola merged commit 1ebdfe7 into hetznercloud:main Jan 9, 2024
6 of 10 checks passed
@hcloud-bot hcloud-bot mentioned this pull request Jan 9, 2024
Merged
@sjagoe sjagoe deleted the f-create-server-existing-public-ip branch January 9, 2024 17:22
jooola pushed a commit that referenced this pull request Jan 12, 2024
@hcloud-bot
chore(main): release 1.3.0 (#137)
06c9b48 
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](v1.2.1...v1.3.0)
(2024-01-09)


### Features

* add labels options to server and ssh keys
([#128](#128))
([3f7dcae](3f7dcae))
* use existing IP for server create
([#144](#144))
([1ebdfe7](1ebdfe7))


### Bug Fixes

* do not pass nil error to error handler
([#145](#145))
([e742263](e742263))
* improve logs messages and error handling
([#139](#139))
([2f2bcf1](2f2bcf1))
* improve missing hcloud token error
([#138](#138))
([e47f476](e47f476))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jooola jooola jooola approved these changes

@apricote apricote Awaiting requested review from apricote

Assignees

@jooola jooola

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sjagoe @jooola @apricote