Skip to content

feat(TPG>=6.28)!: add support for disable_l4_lb_firewall_reconciliation #2338

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
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat(TPG>=6.28)!: add support for disable_l4_lb_firewall_reconciliation #2338

wants to merge 6 commits into from

Conversation

DrFaust92
Copy link
Contributor

No description provided.

@DrFaust92 DrFaust92 requested review from apeabody, ericyz and a team as code owners April 29, 2025 01:53
apeabody
apeabody reviewed Apr 29, 2025
View reviewed changes
@apeabody apeabody changed the title feate: add support for disable_l4_lb_firewall_reconciliation featTPG>=6.28)!: add support for disable_l4_lb_firewall_reconciliation Apr 29, 2025
@apeabody apeabody changed the title featTPG>=6.28)!: add support for disable_l4_lb_firewall_reconciliation feat(TPG>=6.28)!: add support for disable_l4_lb_firewall_reconciliation Apr 29, 2025
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented May 6, 2025 

Step #66 - "apply simple-autopilot-private-local":         	Error:      	Received unexpected error:
Step #66 - "apply simple-autopilot-private-local":         	            	FatalError{Underlying: error while running command: exit status 1; 
Step #66 - "apply simple-autopilot-private-local":         	            	Error: Error updating LinuxNodeConfig: googleapi: Error 400: INVALID_ARGUMENT: invalid node_pool_auto_config.linux_node_config. Allowed fields are: ["cgroup_mode"].
Step #66 - "apply simple-autopilot-private-local":         	            	Details:
Step #66 - "apply simple-autopilot-private-local":         	            	[
Step #66 - "apply simple-autopilot-private-local":         	            	  {
Step #66 - "apply simple-autopilot-private-local":         	            	    "@type": "type.googleapis.com/google.rpc.RequestInfo",
Step #66 - "apply simple-autopilot-private-local":         	            	    "requestId": "0x9a1b30464abe8327"
Step #66 - "apply simple-autopilot-private-local":         	            	  }
Step #66 - "apply simple-autopilot-private-local":         	            	]
Step #66 - "apply simple-autopilot-private-local":         	            	, badRequest
Step #66 - "apply simple-autopilot-private-local":         	            	
Step #66 - "apply simple-autopilot-private-local":         	            	  with module.example.module.gke.google_container_cluster.primary,
Step #66 - "apply simple-autopilot-private-local":         	            	  on ../../../modules/beta-autopilot-private-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
Step #66 - "apply simple-autopilot-private-local":         	            	  22: resource "google_container_cluster" "primary" {
Step #66 - "apply simple-autopilot-private-local":         	            	}
Step #66 - "apply simple-autopilot-private-local":         	Test:       	TestSimpleAutopilotPrivate

@apeabody
Copy link
Collaborator

apeabody commented May 6, 2025 

Step #66 - "apply simple-autopilot-private-local":         	Error:      	Received unexpected error:
Step #66 - "apply simple-autopilot-private-local":         	            	FatalError{Underlying: error while running command: exit status 1; 
Step #66 - "apply simple-autopilot-private-local":         	            	Error: Error updating LinuxNodeConfig: googleapi: Error 400: INVALID_ARGUMENT: invalid node_pool_auto_config.linux_node_config. Allowed fields are: ["cgroup_mode"].
Step #66 - "apply simple-autopilot-private-local":         	            	Details:
Step #66 - "apply simple-autopilot-private-local":         	            	[
Step #66 - "apply simple-autopilot-private-local":         	            	  {
Step #66 - "apply simple-autopilot-private-local":         	            	    "@type": "type.googleapis.com/google.rpc.RequestInfo",
Step #66 - "apply simple-autopilot-private-local":         	            	    "requestId": "0x9a1b30464abe8327"
Step #66 - "apply simple-autopilot-private-local":         	            	  }
Step #66 - "apply simple-autopilot-private-local":         	            	]
Step #66 - "apply simple-autopilot-private-local":         	            	, badRequest
Step #66 - "apply simple-autopilot-private-local":         	            	
Step #66 - "apply simple-autopilot-private-local":         	            	  with module.example.module.gke.google_container_cluster.primary,
Step #66 - "apply simple-autopilot-private-local":         	            	  on ../../../modules/beta-autopilot-private-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
Step #66 - "apply simple-autopilot-private-local":         	            	  22: resource "google_container_cluster" "primary" {
Step #66 - "apply simple-autopilot-private-local":         	            	}
Step #66 - "apply simple-autopilot-private-local":         	Test:       	TestSimpleAutopilotPrivate

That said, I'm not certain this PR makes changes to this area. Will need to verify if this is an emergent issue.

@apeabody
Copy link
Collaborator

apeabody commented May 9, 2025

/gcbrun

1 similar comment
@apeabody
Copy link
Collaborator

apeabody commented May 9, 2025

/gcbrun

@apeabody apeabody self-assigned this May 9, 2025
@apeabody
Copy link
Collaborator

/gcbrun

1 similar comment
@apeabody
Copy link
Collaborator

/gcbrun

@DrFaust92 DrFaust92 requested a review from apeabody May 17, 2025 15:43
@DrFaust92
Copy link
Contributor Author

tested locally, change to null should be fine.

@DrFaust92 DrFaust92 force-pushed the disable_l4_lb_firewall_reconciliation branch from ee6369c to 6239588 Compare May 17, 2025 15:44
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

From the CI tests:

Step #55 - "verify node-pool-local":         	Error:      	Not equal: 
Step #55 - "verify node-pool-local":         	            	expected: "1"
Step #55 - "verify node-pool-local":         	            	actual  : ""
Step #55 - "verify node-pool-local":         	            	
Step #55 - "verify node-pool-local":         	            	Diff:
Step #55 - "verify node-pool-local":         	            	--- Expected
Step #55 - "verify node-pool-local":         	            	+++ Actual
Step #55 - "verify node-pool-local":         	            	@@ -1 +1 @@
Step #55 - "verify node-pool-local":         	            	-1
Step #55 - "verify node-pool-local":         	            	+
Step #55 - "verify node-pool-local":         	Test:       	TestNodePool
Step #55 - "verify node-pool-local":         	Messages:   	For node "pool-04" path "upgradeSettings.maxSurge" expected "" to match fixture "1"

Step #55 - "verify node-pool-local":         	Error:      	Not equal: 
Step #55 - "verify node-pool-local":         	            	expected: "SURGE"
Step #55 - "verify node-pool-local":         	            	actual  : "SHORT_LIVED"
Step #55 - "verify node-pool-local":         	            	
Step #55 - "verify node-pool-local":         	            	Diff:
Step #55 - "verify node-pool-local":         	            	--- Expected
Step #55 - "verify node-pool-local":         	            	+++ Actual
Step #55 - "verify node-pool-local":         	            	@@ -1 +1 @@
Step #55 - "verify node-pool-local":         	            	-SURGE
Step #55 - "verify node-pool-local":         	            	+SHORT_LIVED
Step #55 - "verify node-pool-local":         	Test:       	TestNodePool
Step #55 - "verify node-pool-local":         	Messages:   	For node "pool-04" path "upgradeSettings.strategy" expected "SHORT_LIVED" to match fixture "SURGE"

@DrFaust92
Copy link
Contributor Author

apeabody thats strange, there wasnt any change related to this setting

@apeabody
Copy link
Collaborator

apeabody thats strange, there wasnt any change related to this setting

Hmm - Let me re-run the test, could also be related to the change in the TPG version, API, etc.

@apeabody
Copy link
Collaborator

apeabody thats strange, there wasnt any change related to this setting

Hmm - Let me re-run the test, could also be related to the change in the TPG version, API, etc.

Yup, seeing these on another PR as well.

@apeabody
Copy link
Collaborator

apeabody thats strange, there wasnt any change related to this setting

Hmm - Let me re-run the test, could also be related to the change in the TPG version, API, etc.

Yup, seeing these on another PR as well.

#2352

@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

/gcbrun

1 similar comment
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented May 29, 2025 •
edited
Loading

Hi @DrFaust92 - Can you please run make docker_generate_docs and update the PR. Thanks!

@DrFaust92
regenerate docs
f6c8558 
Signed-off-by: drfaust92 <[email protected]>
@DrFaust92
Copy link
Contributor Author

apeabody done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericyz ericyz Awaiting requested review from ericyz ericyz is a code owner

@apeabody apeabody Awaiting requested review from apeabody apeabody is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@apeabody apeabody

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DrFaust92 @apeabody