Skip to content

fix(server): remove and add nat config back if applicable during resize server job #6675

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

Open
phot0n wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(server): remove and add nat config back if applicable during resize server job #6675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions press/press/doctype/press_job/jobs/resize_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
class ResizeServerJob(PressJob):
@flow
def execute(self):
self.remove_nat_config_if_applicable()
self.stop_virtual_machine()
self.wait_for_virtual_machine_to_stop()

Expand All @@ -28,9 +29,21 @@ def execute(self):
self.wait_for_virtual_machine_to_start()

self.wait_for_server_to_be_accessible()
self.add_nat_config_if_applicable()
self.set_additional_config()
self.increase_disk_size()

@task
def remove_nat_config_if_applicable(self):
if self.server_type not in ("Server", "Database Server") or (
not self.server_doc.nat_server and self.server_doc.ip
):
return
Comment on lines +36 to +41

@greptile-apps greptile-apps Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Edge case in NAT skip guard

The condition not self.server_doc.nat_server and self.server_doc.ip skips NAT operations only when a server has no NAT server and has a direct IP. If both fields are falsy (e.g. a pending or mis-configured server), the AND evaluates to False, so the code proceeds to run the Ansible NAT playbook against a server with no nat_server set — this will likely fail and block the resize. A simpler, safer check is to skip whenever nat_server is not set.

Suggested change
@task
def remove_nat_config_if_applicable(self):
if self.server_type not in ("Server", "Database Server") or (
not self.server_doc.nat_server and self.server_doc.ip
):
return
@task
def remove_nat_config_if_applicable(self):
if self.server_type not in ("Server", "Database Server") or (
not self.server_doc.nat_server
):
return
Prompt To Fix With AI 
This is a comment left during a code review.
Path: press/press/doctype/press_job/jobs/resize_server.py
Line: 36-41

Comment:
**Edge case in NAT skip guard**

The condition `not self.server_doc.nat_server and self.server_doc.ip` skips NAT operations only when a server has no NAT server **and** has a direct IP. If both fields are falsy (e.g. a pending or mis-configured server), the AND evaluates to `False`, so the code proceeds to run the Ansible NAT playbook against a server with no `nat_server` set — this will likely fail and block the resize. A simpler, safer check is to skip whenever `nat_server` is not set.

```suggestion
	@task
	def remove_nat_config_if_applicable(self):
		if self.server_type not in ("Server", "Database Server") or (
			not self.server_doc.nat_server
		):
			return
```

How can I resolve this? If you propose a fix, please make it concise.


play = self.server_doc._remove_nat_iptables()
if not play or play.status != "Success":
raise Exception("Failed to remove NAT configuration")

@task
def stop_virtual_machine(self):
with suppress(Exception):
Expand Down Expand Up @@ -95,6 +108,17 @@ def wait_for_server_to_be_accessible(self):
if not play or play.status != "Success":
self.defer_current_task()

@task
def add_nat_config_if_applicable(self):
if self.server_type not in ("Server", "Database Server") or (
not self.server_doc.nat_server and self.server_doc.ip
):
return
Comment on lines +111 to +116

@greptile-apps greptile-apps Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Same edge-case guard applies to add_nat_config_if_applicable.

Suggested change
@task
def add_nat_config_if_applicable(self):
if self.server_type not in ("Server", "Database Server") or (
not self.server_doc.nat_server and self.server_doc.ip
):
return
@task
def add_nat_config_if_applicable(self):
if self.server_type not in ("Server", "Database Server") or (
not self.server_doc.nat_server
):
return
Prompt To Fix With AI 
This is a comment left during a code review.
Path: press/press/doctype/press_job/jobs/resize_server.py
Line: 111-116

Comment:
Same edge-case guard applies to `add_nat_config_if_applicable`.

```suggestion
	@task
	def add_nat_config_if_applicable(self):
		if self.server_type not in ("Server", "Database Server") or (
			not self.server_doc.nat_server
		):
			return
```

How can I resolve this? If you propose a fix, please make it concise.


@phot0n phot0n Jun 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

maybe i should only add this if it was removed in the previous block?

play = self.server_doc._install_nat_iptables()
if not play or play.status != "Success":
raise Exception("Failed to add NAT configuration")

@task
def set_additional_config(self):
if self.server_type not in ["Server", "Database Server"]:
Expand Down
4 changes: 2 additions & 2 deletions press/press/doctype/server/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2063,7 +2063,7 @@ def _install_nat_iptables(self):
"nat_gateway_ip": self.get_nat_gateway_ip(),
},
)
ansible.run()
return ansible.run()
except Exception:
log_error("NAT Iptables Setup Exception", server=self.as_dict())

Expand All @@ -2079,7 +2079,7 @@ def _remove_nat_iptables(self):
user=self._ssh_user(),
port=self._ssh_port(),
)
ansible.run()
return ansible.run()
except Exception:
log_error("NAT Iptables Removal Exception", server=self.as_dict())

Expand Down
Loading