jenkinsci / google-compute-engine-plugin

https://plugins.jenkins.io/google-compute-engine/
Apache License 2.0
57 stars 88 forks source link

External IP Address can't be configured (dual-stack mode) #440

Closed Artmorse closed 9 months ago

Artmorse commented 9 months ago

This PR wants to fix the issue #438 introduced in #434.

Reproduction steps:

  1. Add a Cloud Instance configuration, making sure to expand ‘Advanced’ and choose Network IP Stack Type of IPV4_ONLY and also choose “Attach External IPV4 address”
  2. After saving the instance, edit the configuration again and note that “Attach External IPV4 Address” is now unchecked
  3. Create a node using this configuration, note that it does not have an external IP address

Fix

The issue came from the readResolve method. Each time we saved the configuration we passed in the readResolve method which reset the networkInterfaceIpStackMode to NetworkInterfaceSingleStack(externalAddress). I set the externalAddress as a primitive Boolean and I added a condition in the readResolve to set this value to null after the first time we reach it.

Testing done

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [ ] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue
batmat commented 9 months ago

@BrianRossmajerAxonify @Vitorspk @bandi13 could you please try this change out and let us know whether this fixes the issue you were seeing (and you don't see anything else broken :)).

The release that contains this fix is https://github.com/jenkinsci/google-compute-engine-plugin/releases/tag/4.561.vfc075cc926e3.

Thank you very much!

BrianRossmajerAxonify commented 9 months ago

At first glance, it looks like it fixes the issue I was seeing.

I didn't confirm that I could change the setting in the UI and see it saved properly; I might be able to test that tomorrow, but in the meantime it's looking much better -- very much appreciate your efforts!

batmat commented 9 months ago

Awesome, thanks for confirming!

bandi13 commented 9 months ago

Can confirm this has been fixed.