Make the "port forwarding create" command idempotent #89
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
After this merges we should push a new release to pypi. |
Member
Author
|
Curses, need to update the tests. |
larsks
force-pushed
the
feature/idempotent-forwarding
branch
from
06f5b07 to
9caf7f0
Compare
Member
Author
|
Tests are updated! |
tzumainn
reviewed
May 19, 2025
Contributor
tzumainn
left a comment
tzumainn
left a comment
There was a problem hiding this comment.
Works well - two minor comments/questions!
larsks
force-pushed
the
feature/idempotent-forwarding
branch
from
9caf7f0 to
f669899
Compare
The `port forwarding create` command would exit with an error
if you attempted to create a forwarding configuration that already exists.
This made it difficult to identify actual failures.
This commit updates the `port forwarding create` command so that it is not
an error if the requested forwarding already exists.
The old behavior was:
$ openstack esi port forwarding create 192.168.11.31 128.31.20.118 -p 8888
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
| ID | Internal Port | External Port | Protocol | Internal IP | External IP |
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
| 43c96be3-861d-4818-a32a-079a93b6f9c4 | 8888 | 8888 | tcp | 192.168.11.31 | 128.31.20.118 |
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
$ openstack esi port forwarding create esi-MOC-R4PAC24U35-S1B-provisioning 128.31.20.118 -p 8888 -p 8889
BadRequestException: 400: Client Error for url:
https://esi.massopen.cloud:13696/v2.0/floatingips/be4b2718-ec5f-4722-80b5-485c421e4a72/port_forwardings,
Bad port_forwarding request: A duplicate port forwarding entry with
same attributes already exists, conflicting values are
{'floatingip_id': 'be4b2718-ec5f-4722-80b5-485c421e4a72',
'external_port': 8888, 'protocol': 'tcp'}.
The new behavior looks like this:
$ openstack esi port forwarding create 192.168.11.31 128.31.20.118 -p 8888
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
| ID | Internal Port | External Port | Protocol | Internal IP | External IP |
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
| 43c96be3-861d-4818-a32a-079a93b6f9c4 | 8888 | 8888 | tcp | 192.168.11.31 | 128.31.20.118 |
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
$ openstack esi port forwarding create esi-MOC-R4PAC24U35-S1B-provisioning 128.31.20.118 -p 8888 -p 8889
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
| ID | Internal Port | External Port | Protocol | Internal IP | External IP |
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
| exists | 8888 | 8888 | tcp | 192.168.11.31 | 128.31.20.118 |
| d7d56201-232a-41fe-8ae6-1f6fb88a37d3 | 8889 | 8889 | tcp | 192.168.11.31 | 128.31.20.118 |
+--------------------------------------+---------------+---------------+----------+---------------+---------------+
larsks
force-pushed
the
feature/idempotent-forwarding
branch
from
f669899 to
900d169
Compare
tzumainn
approved these changes
May 19, 2025
Contributor
tzumainn
left a comment
tzumainn
left a comment
There was a problem hiding this comment.
Works perfect - thanks!
Contributor
|
new release pushed to pypi |
larsks
added a commit
to larsks/cloudkit-aap
that referenced
this pull request
May 19, 2025
We use the `openstack esi port forwarding` command in the `external_access` role to configure port forwarding. Previously, this command would fail with an error when attempting to create a port forwarding that already exists. This made it difficult to identify legitimate failures. With CCI-MOC/python-esiclient#89, python-esiclient is now idempotent and will not emit an error if an existing port forwarding exactly matches the requested configuration.
larsks
added a commit
to innabox/cloudkit-aap
that referenced
this pull request
May 19, 2025
We use the `openstack esi port forwarding` command in the `external_access` role to configure port forwarding. Previously, this command would fail with an error when attempting to create a port forwarding that already exists. This made it difficult to identify legitimate failures. With CCI-MOC/python-esiclient#89, python-esiclient is now idempotent and will not emit an error if an existing port forwarding exactly matches the requested configuration.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
port forwarding createcommand would exit with an error if you attempted to create a forwarding configuration that already exists. This made it difficult to identify actual failures.This commit updates the
port forwarding createcommand so that it is not an error if the requested forwarding already exists.The old behavior was:
The new behavior looks like this: