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.

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

Fix : Wrong control_plane_is_public behavior for OKE cluster #888

Merged
merged 3 commits into from
Mar 3, 2024

Conversation

syedthameem85
Copy link
Member

@syedthameem85 syedthameem85 commented Jan 25, 2024

Resolves[https://github.com//issues/869]

@Noksa / @hyder / @devoncrouse
With public subnets, we have option to either assign public IP or defer it for assignment at a later point of time. This is the feature that is being used to assign or unassign the public IP to Control plane API endpoint in OCI console. So if the cluster has been created with public subnet with/without actually assigning the public IP at the time off creation, there should be an option to toggle the public IP after cluster is provisioned.

The variable control_plane_is_public is being used in code to decide the subnet type (public or private). If we toggle this variable, the subnet recreation will kick off which will recreate the cluster and nodepools.

For solution to toggle the public ip , we need to introduce a new boolean variable (assign_public_ip_to_control_plane) to toggle public ip on control plan API endpoint.

With control_plane_is_public set to true, assign_public_ip_to_control_plane can be set to either true or false.

When assign_public_ip_to_control_plane is set to true, the public IP will be assigned to Control plane API endpoint.
When assign_public_ip_to_control_plane set to false, although the subnet is public, the public IP will not be assigned to the API endpoint.
With control_plane_is_public set to false, public ip will not be allocated to API endpoint irrespective of value set for assign_public_ip_to_control_plane

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jan 25, 2024
@syedthameem85 syedthameem85 changed the title bug # 869 Wrong control_plane_is_public behavior for OKE cluster #869 Jan 25, 2024
@hyder hyder linked an issue Feb 6, 2024 that may be closed by this pull request
@hyder
Copy link
Contributor

hyder commented Feb 6, 2024

What is the subnet visibility and behaviour when doing this swap in the OCI console?

@syedthameem85
Copy link
Member Author

What is the subnet visibility and behaviour when doing this swap in the OCI console?

@hyder - The public IP toggle IP option is available for Kubernetes API Endpoint subnet. The OCI console forces user to provide public subnet if public IP assignment is checked.

@hyder
Copy link
Contributor

hyder commented Feb 6, 2024

Why not make the control plane subnet public anyway and then use the existing variable to decide whether to allocate a public IP or not?

@syedthameem85
Copy link
Member Author

Why not make the control plane subnet public anyway and then use the existing variable to decide whether to allocate a public IP or not?

This cannot be achieved with one variable. If you are saying that force the control plane subnet to be public always, this would not be inline with console. When you set control_plane_is_public to true, how would we decide the case where subnet is public while public IP is not required?

@hyder
Copy link
Contributor

hyder commented Feb 8, 2024

A public subnet can have both public and private IP addresses.

If the idea is be able to switch between a publicly accessible control plane and a private one, then you use the existing variable and keep the subnet public. Otherwise, this code is redundant:

is_public_ip_enabled = var.control_plane_is_public ? var.assign_public_ip_to_control_plane : var.control_plane_is_public

@syedthameem85
Copy link
Member Author

@hyder - We have 2 conditions that need to be addressed here - first condition is whether the subnet is public or private and when subnet is public, whether we need to assign public ip or not. To serve these 2 conditions, we need to have 2 variables.

@hyder
Copy link
Contributor

hyder commented Feb 15, 2024

@hyder - We have 2 conditions that need to be addressed here - first condition is whether the subnet is public or private and when subnet is public, whether we need to assign public IP or not. To serve these 2 conditions, we need to have 2 variables.

I don't see the need for 2 variables here. The only reason you want to make the subnet public is to have a public IP address. Since a public subnet can have both public and private IPs and the intent is to be able to toggle the visibility of the control plane, then use the existing variable to toggle the visibility instead of the subnet.

What is the impact on routing table, use of DRG for private connectivity to the control plane?

@12345ieee
Copy link
Contributor

He has the same issue I have, which is not at creation time, but at update time.

Say I created a cluster with public endpoint (hence a public subnet), then I want it to become private.
On the console I'd just change the cluster endpoint type and be done.

In this module though I have it coupled with the subnet, and this puts me in a deadlock:

  • Subnets can't be updated to be private, only recreated
  • I cannot destroy the subnet without also destroying the cluster
  • I don't want the cluster to go down

Having 2 booleans for the 2 private/public types solves it (clearly only 3 configurations are possible, the other should be asserted away).

It's never been a priority big enough for me to PR it, but seemed a good moment now.

@syedthameem85
Copy link
Member Author

syedthameem85 commented Feb 15, 2024

@hyder This code is inline with OCI console behavior. When someone toggles the Public IP for API endpoint in OCI console, its doesn't go ahead and recreate the subnet (and it should not do that). The subnet still stays public and its just the IP that is toggled. So, there shouldn't be any changes required at network level (be it routing table or DRG).

@syedthameem85 syedthameem85 changed the title Wrong control_plane_is_public behavior for OKE cluster #869 Resolves #869 - Wrong control_plane_is_public behavior for OKE cluster Feb 21, 2024
variables-cluster.tf Outdated Show resolved Hide resolved
modules/cluster/cluster.tf Outdated Show resolved Hide resolved
Copy link
Contributor

@hyder hyder left a comment

Choose a reason for hiding this comment

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

Please also add the variable in the vars example for cluster and in the documentation

@syedthameem85
Copy link
Member Author

Please also add the variable in the vars example for cluster and in the documentation

@hyder - Done

@@ -16,7 +16,7 @@ resource "oci_containerengine_cluster" "k8s_cluster" {
}

endpoint_config {
is_public_ip_enabled = var.control_plane_is_public
is_public_ip_enabled = var.control_plane_is_public && var.assign_public_ip_to_control_planeßß
Copy link
Contributor

Choose a reason for hiding this comment

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

you have extra characters at the end of this line.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hyder - Done

Copy link
Contributor

@hyder hyder left a comment

Choose a reason for hiding this comment

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

please remove extra characters

@syedthameem85 syedthameem85 changed the title Resolves #869 - Wrong control_plane_is_public behavior for OKE cluster # 888 Fix : Wrong control_plane_is_public behavior for OKE cluster Feb 28, 2024
@syedthameem85 syedthameem85 changed the title # 888 Fix : Wrong control_plane_is_public behavior for OKE cluster Fix : Wrong control_plane_is_public behavior for OKE cluster Feb 28, 2024
Copy link
Contributor

@hyder hyder left a comment

Choose a reason for hiding this comment

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

approved

@hyder hyder merged commit 10b2ae4 into oracle-terraform-modules:main Mar 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong control_plane_is_public behavior for OKE cluster
3 participants