[PATCH 1/3] Set kdevops_terraform_ssh_config_privkey_file unconditionally

[PATCH 1/3] Set kdevops_terraform_ssh_config_privkey_file unconditionally

[Chandan Babu R]

kdevops_terraform_ssh_config_privkey_file is not set on control systems which
use pre-existing ssh keys. Hence, adding entries of provisioned cloud
instances to the ephemeral ssh config file fails on such systems.

Fix this by setting kdevops_terraform_ssh_config_privkey_file
unconditionally.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 scripts/terraform.Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/terraform.Makefile b/scripts/terraform.Makefile
index 91469469..b6b26e69 100644
--- a/scripts/terraform.Makefile
+++ b/scripts/terraform.Makefile
@@ -85,10 +85,10 @@ endif # CONFIG_KDEVOPS_SSH_CONFIG_UPDATE
 export KDEVOPS_SSH_PUBKEY:=$(shell realpath $(subst ",,$(CONFIG_TERRAFORM_SSH_CONFIG_PUBKEY_FILE)))
 TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_pubkey_file='$(KDEVOPS_SSH_PUBKEY)'
 TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_user='$(SSH_CONFIG_USER)'
+TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_privkey_file='$(basename $(KDEVOPS_SSH_PUBKEY))'
 
 ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY))
 export KDEVOPS_SSH_PRIVKEY:=$(basename $(KDEVOPS_SSH_PUBKEY))
-TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_privkey_file='$(KDEVOPS_SSH_PRIVKEY)'
 
 ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY_OVERWRITE))
 DEFAULT_DEPS += remove-ssh-key
-- 
2.45.2

[PATCH 2/3] Remove kdevops_terraform_ssh_config_genkey_overwrite

[Chandan Babu R]

Adding entries to the ephemeral ssh config file on control systems is being
done only when kdevops_terraform_ssh_config_genkey_overwrite evaluates to
true. This is incorrect because we need to allow Kdevops to be used with
pre-existing ssh keys. Hence remove the conditions in
playbooks/roles/terraform/defaults/main.yml which check the value of
kdevops_terraform_ssh_config_genkey_overwrite.

Also, the value of kdevops_terraform_ssh_config_genkey_overwrite is not
checked in any other file. Hence this commit removes the remaining instances
of this variable.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 playbooks/roles/terraform/defaults/main.yml | 1 -
 playbooks/roles/terraform/tasks/main.yml    | 8 --------
 scripts/terraform.Makefile                  | 1 -
 3 files changed, 10 deletions(-)

diff --git a/playbooks/roles/terraform/defaults/main.yml b/playbooks/roles/terraform/defaults/main.yml
index f3ef9b18..33bd00e6 100644
--- a/playbooks/roles/terraform/defaults/main.yml
+++ b/playbooks/roles/terraform/defaults/main.yml
@@ -1,3 +1,2 @@
 ---
 ssh_config_kexalgorithms: ""
-kdevops_terraform_ssh_config_genkey_overwrite: false
diff --git a/playbooks/roles/terraform/tasks/main.yml b/playbooks/roles/terraform/tasks/main.yml
index 272ebf93..d04dc997 100644
--- a/playbooks/roles/terraform/tasks/main.yml
+++ b/playbooks/roles/terraform/tasks/main.yml
@@ -15,8 +15,6 @@
     cmd: "terraform output -json public_ip_map"
   register: terraform_output
   changed_when: false
-  when:
-    - kdevops_terraform_ssh_config_genkey_overwrite|bool
   tags:
     - ssh
 
@@ -25,8 +23,6 @@
   run_once: true
   ansible.builtin.set_fact:
     public_ip_map: "{{ terraform_output.stdout | from_json }}"
-  when:
-    - kdevops_terraform_ssh_config_genkey_overwrite|bool
   tags:
     - ssh
 
@@ -40,8 +36,6 @@
     insertafter: "EOF"
     marker: "# {mark} host configuration for {{ inventory_hostname }}"
     mode: "u=rw,g=r,o=r"
-  when:
-    - kdevops_terraform_ssh_config_genkey_overwrite|bool
   tags:
     - ssh
 
@@ -55,8 +49,6 @@
     marker: "# {mark} Managed by kdevops"
     mode: "u=rw,g=r,o=r"
     block: "Include {{ kdevops_ssh_config_prefix }}*"
-  when:
-    - kdevops_terraform_ssh_config_genkey_overwrite|bool
   tags:
     - ssh
 
diff --git a/scripts/terraform.Makefile b/scripts/terraform.Makefile
index b6b26e69..865c84c3 100644
--- a/scripts/terraform.Makefile
+++ b/scripts/terraform.Makefile
@@ -92,7 +92,6 @@ export KDEVOPS_SSH_PRIVKEY:=$(basename $(KDEVOPS_SSH_PUBKEY))
 
 ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY_OVERWRITE))
 DEFAULT_DEPS += remove-ssh-key
-TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_genkey_overwrite='True'
 endif
 
 DEFAULT_DEPS += $(KDEVOPS_SSH_PRIVKEY)
-- 
2.45.2

[PATCH 3/3] Allow private IP address to be assigned to instances created on OCI

[Chandan Babu R]

The OCI tenancy available to kernel developers from Oracle allows only private
IP addresses to be assigned to compute instances. Hence this commit brings
back the ability to work with OCI instances having private IP addresses.

The variable "public_ip_map" defined in terraform/*/output.tf files has also
been changed to "ansible_ip_map" in order to abstract away the type of the IP
address being used.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---

Please note that since I have only an OCI account, I haven't tested
the changes on other cloud providers. However, the changes are limited
to renaming "public_ip_map" to "ansible_ip_map" in terraform files.

 playbooks/roles/terraform/tasks/main.yml          |  8 ++++----
 playbooks/roles/terraform/templates/ssh_config.j2 |  4 ++--
 scripts/status_terraform.sh                       |  2 +-
 terraform/aws/output.tf                           |  2 +-
 terraform/azure/output.tf                         |  2 +-
 terraform/gce/output.tf                           |  2 +-
 terraform/oci/output.tf                           | 14 +++++++++-----
 terraform/openstack/output.tf                     |  2 +-
 8 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/playbooks/roles/terraform/tasks/main.yml b/playbooks/roles/terraform/tasks/main.yml
index d04dc997..d1c94264 100644
--- a/playbooks/roles/terraform/tasks/main.yml
+++ b/playbooks/roles/terraform/tasks/main.yml
@@ -7,22 +7,22 @@
   tags:
     - bringup
 
-- name: Retrieve the public_ip_map from terraform
+- name: Retrieve the ansible_ip_map from terraform
   delegate_to: localhost
   run_once: true
   ansible.builtin.command:
     chdir: "{{ topdir_path }}/terraform/{{ kdevops_terraform_provider }}"
-    cmd: "terraform output -json public_ip_map"
+    cmd: "terraform output -json ansible_ip_map"
   register: terraform_output
   changed_when: false
   tags:
     - ssh
 
-- name: Convert the retrieved public_ip_map into a dictionary
+- name: Convert the retrieved ansible_ip_map into a dictionary
   delegate_to: localhost
   run_once: true
   ansible.builtin.set_fact:
-    public_ip_map: "{{ terraform_output.stdout | from_json }}"
+    ansible_ip_map: "{{ terraform_output.stdout | from_json }}"
   tags:
     - ssh
 
diff --git a/playbooks/roles/terraform/templates/ssh_config.j2 b/playbooks/roles/terraform/templates/ssh_config.j2
index d9b057b1..c6e62a61 100644
--- a/playbooks/roles/terraform/templates/ssh_config.j2
+++ b/playbooks/roles/terraform/templates/ssh_config.j2
@@ -1,5 +1,5 @@
-Host {{ inventory_hostname }} {{ public_ip_map[inventory_hostname] }}
-	HostName {{ public_ip_map[inventory_hostname] }}
+Host {{ inventory_hostname }} {{ ansible_ip_map[inventory_hostname] }}
+	HostName {{ ansible_ip_map[inventory_hostname] }}
 	User {{ kdevops_terraform_ssh_config_user }}
 	Port 22
 	IdentityFile {{ kdevops_terraform_ssh_config_privkey_file }}
diff --git a/scripts/status_terraform.sh b/scripts/status_terraform.sh
index dc82d79e..bc4ad309 100755
--- a/scripts/status_terraform.sh
+++ b/scripts/status_terraform.sh
@@ -13,5 +13,5 @@ if [[ ${COUNT} -eq 1 ]]; then
 else
   echo "There are ${COUNT} active terraform resources on '$1'."
 fi
-terraform output public_ip_map
+terraform output ansible_ip_map
 exit 0
diff --git a/terraform/aws/output.tf b/terraform/aws/output.tf
index ab18bc55..b5e59eb1 100644
--- a/terraform/aws/output.tf
+++ b/terraform/aws/output.tf
@@ -3,7 +3,7 @@
 # Each provider's output.tf needs to define a public_ip_map. This
 # map is used to build the Ansible controller's ssh configuration.
 # Each map entry contains the node's hostname and public IP address.
-output "public_ip_map" {
+output "ansible_ip_map" {
   description = "The public IP addresses assigned to each instance"
   value       = zipmap(var.kdevops_nodes[*], aws_eip.kdevops_eip[*].public_ip)
 }
diff --git a/terraform/azure/output.tf b/terraform/azure/output.tf
index 38104c90..deb7d141 100644
--- a/terraform/azure/output.tf
+++ b/terraform/azure/output.tf
@@ -1,7 +1,7 @@
 # Each provider's output.tf needs to define a public_ip_map. This
 # map is used to build the Ansible controller's ssh configuration.
 # Each map entry contains the node's hostname and public IP address.
-output "public_ip_map" {
+output "ansible_ip_map" {
   description = "The public IP addresses assigned to each instance"
   value       = zipmap(var.kdevops_nodes[*], azurerm_public_ip.kdevops_publicip[*].ip_address)
 }
diff --git a/terraform/gce/output.tf b/terraform/gce/output.tf
index 15889cc4..9246216f 100644
--- a/terraform/gce/output.tf
+++ b/terraform/gce/output.tf
@@ -3,7 +3,7 @@
 # Each provider's output.tf needs to define a public_ip_map. This
 # map is used to build the Ansible controller's ssh configuration.
 # Each map entry contains the node's hostname and public IP address.
-output "public_ip_map" {
+output "ansible_ip_map" {
   description = "The public IP addresses assigned to each instance"
   value = zipmap(var.kdevops_nodes[*],
   google_compute_instance.kdevops_instance[*].network_interface[0].access_config[0].nat_ip)
diff --git a/terraform/oci/output.tf b/terraform/oci/output.tf
index 1657b804..6562a1e8 100644
--- a/terraform/oci/output.tf
+++ b/terraform/oci/output.tf
@@ -1,9 +1,13 @@
 # All generic output goes here
 
-# Each provider's output.tf needs to define a public_ip_map. This
+# Each provider's output.tf needs to define a ansible_ip_map. This
 # map is used to build the Ansible controller's ssh configuration.
-# Each map entry contains the node's hostname and public IP address.
-output "public_ip_map" {
-  description = "The public IP addresses assigned to each instance"
-  value       = zipmap(var.kdevops_nodes[*], oci_core_instance.kdevops_instance.*.public_ip)
+# Each map entry contains the node's hostname and public/private IP
+# address.
+output "ansible_ip_map" {
+  description = "The IP addresses assigned to each instance"
+  value       = zipmap(var.kdevops_nodes[*],
+    var.oci_assign_public_ip ?
+    oci_core_instance.kdevops_instance.*.public_ip :
+    oci_core_instance.kdevops_instance.*.private_ip)
 }
diff --git a/terraform/openstack/output.tf b/terraform/openstack/output.tf
index d5d3a6b1..3dd139a3 100644
--- a/terraform/openstack/output.tf
+++ b/terraform/openstack/output.tf
@@ -21,7 +21,7 @@ output "kdevops_hosts_and_ipv4" {
 # Each provider's output.tf needs to define a public_ip_map. This
 # map is used to build the Ansible controller's ssh configuration.
 # Each map entry contains the node's hostname and public IP address.
-output "public_ip_map" {
+output "ansible_ip_map" {
   description = "The public IP addresses assigned to each instance"
   value       = zipmap(var.kdevops_nodes[*], openstack_compute_instance_v2.kdevops_instances[*].access_ip_v4)
 }
-- 
2.45.2

Re: [PATCH 3/3] Allow private IP address to be assigned to instances created on OCI

[Chuck Lever]

On 5/2/25 2:46 AM, Chandan Babu R wrote:
> The OCI tenancy available to kernel developers from Oracle allows only private
> IP addresses to be assigned to compute instances. Hence this commit brings
> back the ability to work with OCI instances having private IP addresses.
> 
> The variable "public_ip_map" defined in terraform/*/output.tf files has also
> been changed to "ansible_ip_map" in order to abstract away the type of the IP
> address being used.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
> 
> Please note that since I have only an OCI account, I haven't tested
> the changes on other cloud providers. However, the changes are limited
> to renaming "public_ip_map" to "ansible_ip_map" in terraform files.
> 
>  playbooks/roles/terraform/tasks/main.yml          |  8 ++++----
>  playbooks/roles/terraform/templates/ssh_config.j2 |  4 ++--
>  scripts/status_terraform.sh                       |  2 +-
>  terraform/aws/output.tf                           |  2 +-
>  terraform/azure/output.tf                         |  2 +-
>  terraform/gce/output.tf                           |  2 +-
>  terraform/oci/output.tf                           | 14 +++++++++-----
>  terraform/openstack/output.tf                     |  2 +-
>  8 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/playbooks/roles/terraform/tasks/main.yml b/playbooks/roles/terraform/tasks/main.yml
> index d04dc997..d1c94264 100644
> --- a/playbooks/roles/terraform/tasks/main.yml
> +++ b/playbooks/roles/terraform/tasks/main.yml
> @@ -7,22 +7,22 @@
>    tags:
>      - bringup
>  
> -- name: Retrieve the public_ip_map from terraform
> +- name: Retrieve the ansible_ip_map from terraform
>    delegate_to: localhost
>    run_once: true
>    ansible.builtin.command:
>      chdir: "{{ topdir_path }}/terraform/{{ kdevops_terraform_provider }}"
> -    cmd: "terraform output -json public_ip_map"
> +    cmd: "terraform output -json ansible_ip_map"
>    register: terraform_output
>    changed_when: false
>    tags:
>      - ssh
>  
> -- name: Convert the retrieved public_ip_map into a dictionary
> +- name: Convert the retrieved ansible_ip_map into a dictionary
>    delegate_to: localhost
>    run_once: true
>    ansible.builtin.set_fact:
> -    public_ip_map: "{{ terraform_output.stdout | from_json }}"
> +    ansible_ip_map: "{{ terraform_output.stdout | from_json }}"
>    tags:
>      - ssh
>  
> diff --git a/playbooks/roles/terraform/templates/ssh_config.j2 b/playbooks/roles/terraform/templates/ssh_config.j2
> index d9b057b1..c6e62a61 100644
> --- a/playbooks/roles/terraform/templates/ssh_config.j2
> +++ b/playbooks/roles/terraform/templates/ssh_config.j2
> @@ -1,5 +1,5 @@
> -Host {{ inventory_hostname }} {{ public_ip_map[inventory_hostname] }}
> -	HostName {{ public_ip_map[inventory_hostname] }}
> +Host {{ inventory_hostname }} {{ ansible_ip_map[inventory_hostname] }}
> +	HostName {{ ansible_ip_map[inventory_hostname] }}
>  	User {{ kdevops_terraform_ssh_config_user }}
>  	Port 22
>  	IdentityFile {{ kdevops_terraform_ssh_config_privkey_file }}
> diff --git a/scripts/status_terraform.sh b/scripts/status_terraform.sh
> index dc82d79e..bc4ad309 100755
> --- a/scripts/status_terraform.sh
> +++ b/scripts/status_terraform.sh
> @@ -13,5 +13,5 @@ if [[ ${COUNT} -eq 1 ]]; then
>  else
>    echo "There are ${COUNT} active terraform resources on '$1'."
>  fi
> -terraform output public_ip_map
> +terraform output ansible_ip_map
>  exit 0
> diff --git a/terraform/aws/output.tf b/terraform/aws/output.tf
> index ab18bc55..b5e59eb1 100644
> --- a/terraform/aws/output.tf
> +++ b/terraform/aws/output.tf
> @@ -3,7 +3,7 @@
>  # Each provider's output.tf needs to define a public_ip_map. This
>  # map is used to build the Ansible controller's ssh configuration.
>  # Each map entry contains the node's hostname and public IP address.
> -output "public_ip_map" {
> +output "ansible_ip_map" {
>    description = "The public IP addresses assigned to each instance"
>    value       = zipmap(var.kdevops_nodes[*], aws_eip.kdevops_eip[*].public_ip)
>  }
> diff --git a/terraform/azure/output.tf b/terraform/azure/output.tf
> index 38104c90..deb7d141 100644
> --- a/terraform/azure/output.tf
> +++ b/terraform/azure/output.tf
> @@ -1,7 +1,7 @@
>  # Each provider's output.tf needs to define a public_ip_map. This
>  # map is used to build the Ansible controller's ssh configuration.
>  # Each map entry contains the node's hostname and public IP address.
> -output "public_ip_map" {
> +output "ansible_ip_map" {
>    description = "The public IP addresses assigned to each instance"
>    value       = zipmap(var.kdevops_nodes[*], azurerm_public_ip.kdevops_publicip[*].ip_address)
>  }
> diff --git a/terraform/gce/output.tf b/terraform/gce/output.tf
> index 15889cc4..9246216f 100644
> --- a/terraform/gce/output.tf
> +++ b/terraform/gce/output.tf
> @@ -3,7 +3,7 @@
>  # Each provider's output.tf needs to define a public_ip_map. This
>  # map is used to build the Ansible controller's ssh configuration.
>  # Each map entry contains the node's hostname and public IP address.
> -output "public_ip_map" {
> +output "ansible_ip_map" {
>    description = "The public IP addresses assigned to each instance"
>    value = zipmap(var.kdevops_nodes[*],
>    google_compute_instance.kdevops_instance[*].network_interface[0].access_config[0].nat_ip)
> diff --git a/terraform/oci/output.tf b/terraform/oci/output.tf
> index 1657b804..6562a1e8 100644
> --- a/terraform/oci/output.tf
> +++ b/terraform/oci/output.tf
> @@ -1,9 +1,13 @@
>  # All generic output goes here
>  
> -# Each provider's output.tf needs to define a public_ip_map. This
> +# Each provider's output.tf needs to define a ansible_ip_map. This
>  # map is used to build the Ansible controller's ssh configuration.
> -# Each map entry contains the node's hostname and public IP address.
> -output "public_ip_map" {
> -  description = "The public IP addresses assigned to each instance"
> -  value       = zipmap(var.kdevops_nodes[*], oci_core_instance.kdevops_instance.*.public_ip)
> +# Each map entry contains the node's hostname and public/private IP
> +# address.

Can you update the documenting comment in the output.tf file for the
other providers as well?


> +output "ansible_ip_map" {
> +  description = "The IP addresses assigned to each instance"

Also, update the description for the ansible_ip_map output variable in
all the output.tf files.


> +  value       = zipmap(var.kdevops_nodes[*],
> +    var.oci_assign_public_ip ?
> +    oci_core_instance.kdevops_instance.*.public_ip :
> +    oci_core_instance.kdevops_instance.*.private_ip)

Weird indentation. I assume this was the result of "terraform fmt" ?


>  }
> diff --git a/terraform/openstack/output.tf b/terraform/openstack/output.tf
> index d5d3a6b1..3dd139a3 100644
> --- a/terraform/openstack/output.tf
> +++ b/terraform/openstack/output.tf
> @@ -21,7 +21,7 @@ output "kdevops_hosts_and_ipv4" {
>  # Each provider's output.tf needs to define a public_ip_map. This
>  # map is used to build the Ansible controller's ssh configuration.
>  # Each map entry contains the node's hostname and public IP address.
> -output "public_ip_map" {
> +output "ansible_ip_map" {
>    description = "The public IP addresses assigned to each instance"
>    value       = zipmap(var.kdevops_nodes[*], openstack_compute_instance_v2.kdevops_instances[*].access_ip_v4)
>  }

This patch makes sense to me, just needs a little polish.


-- 
Chuck Lever

Re: [PATCH 2/3] Remove kdevops_terraform_ssh_config_genkey_overwrite

[Chuck Lever]

On 5/2/25 2:46 AM, Chandan Babu R wrote:
> Adding entries to the ephemeral ssh config file on control systems is being
> done only when kdevops_terraform_ssh_config_genkey_overwrite evaluates to
> true. This is incorrect because we need to allow Kdevops to be used with
> pre-existing ssh keys. Hence remove the conditions in
> playbooks/roles/terraform/defaults/main.yml which check the value of
> kdevops_terraform_ssh_config_genkey_overwrite.
> 
> Also, the value of kdevops_terraform_ssh_config_genkey_overwrite is not
> checked in any other file. Hence this commit removes the remaining instances
> of this variable.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
>  playbooks/roles/terraform/defaults/main.yml | 1 -
>  playbooks/roles/terraform/tasks/main.yml    | 8 --------
>  scripts/terraform.Makefile                  | 1 -
>  3 files changed, 10 deletions(-)
> 
> diff --git a/playbooks/roles/terraform/defaults/main.yml b/playbooks/roles/terraform/defaults/main.yml
> index f3ef9b18..33bd00e6 100644
> --- a/playbooks/roles/terraform/defaults/main.yml
> +++ b/playbooks/roles/terraform/defaults/main.yml
> @@ -1,3 +1,2 @@
>  ---
>  ssh_config_kexalgorithms: ""
> -kdevops_terraform_ssh_config_genkey_overwrite: false
> diff --git a/playbooks/roles/terraform/tasks/main.yml b/playbooks/roles/terraform/tasks/main.yml
> index 272ebf93..d04dc997 100644
> --- a/playbooks/roles/terraform/tasks/main.yml
> +++ b/playbooks/roles/terraform/tasks/main.yml
> @@ -15,8 +15,6 @@
>      cmd: "terraform output -json public_ip_map"
>    register: terraform_output
>    changed_when: false
> -  when:
> -    - kdevops_terraform_ssh_config_genkey_overwrite|bool
>    tags:
>      - ssh
>  
> @@ -25,8 +23,6 @@
>    run_once: true
>    ansible.builtin.set_fact:
>      public_ip_map: "{{ terraform_output.stdout | from_json }}"
> -  when:
> -    - kdevops_terraform_ssh_config_genkey_overwrite|bool
>    tags:
>      - ssh
>  
> @@ -40,8 +36,6 @@
>      insertafter: "EOF"
>      marker: "# {mark} host configuration for {{ inventory_hostname }}"
>      mode: "u=rw,g=r,o=r"
> -  when:
> -    - kdevops_terraform_ssh_config_genkey_overwrite|bool
>    tags:
>      - ssh
>  
> @@ -55,8 +49,6 @@
>      marker: "# {mark} Managed by kdevops"
>      mode: "u=rw,g=r,o=r"
>      block: "Include {{ kdevops_ssh_config_prefix }}*"
> -  when:
> -    - kdevops_terraform_ssh_config_genkey_overwrite|bool
>    tags:
>      - ssh
>  
> diff --git a/scripts/terraform.Makefile b/scripts/terraform.Makefile
> index b6b26e69..865c84c3 100644
> --- a/scripts/terraform.Makefile
> +++ b/scripts/terraform.Makefile
> @@ -92,7 +92,6 @@ export KDEVOPS_SSH_PRIVKEY:=$(basename $(KDEVOPS_SSH_PUBKEY))
>  
>  ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY_OVERWRITE))
>  DEFAULT_DEPS += remove-ssh-key
> -TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_genkey_overwrite='True'
>  endif
>  
>  DEFAULT_DEPS += $(KDEVOPS_SSH_PRIVKEY)

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

-- 
Chuck Lever

Re: [PATCH 1/3] Set kdevops_terraform_ssh_config_privkey_file unconditionally

[Chuck Lever]

On 5/2/25 2:46 AM, Chandan Babu R wrote:
> kdevops_terraform_ssh_config_privkey_file is not set on control systems which
> use pre-existing ssh keys. Hence, adding entries of provisioned cloud
> instances to the ephemeral ssh config file fails on such systems.
> 
> Fix this by setting kdevops_terraform_ssh_config_privkey_file
> unconditionally.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
>  scripts/terraform.Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/terraform.Makefile b/scripts/terraform.Makefile
> index 91469469..b6b26e69 100644
> --- a/scripts/terraform.Makefile
> +++ b/scripts/terraform.Makefile
> @@ -85,10 +85,10 @@ endif # CONFIG_KDEVOPS_SSH_CONFIG_UPDATE
>  export KDEVOPS_SSH_PUBKEY:=$(shell realpath $(subst ",,$(CONFIG_TERRAFORM_SSH_CONFIG_PUBKEY_FILE)))
>  TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_pubkey_file='$(KDEVOPS_SSH_PUBKEY)'
>  TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_user='$(SSH_CONFIG_USER)'
> +TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_privkey_file='$(basename $(KDEVOPS_SSH_PUBKEY))'
>  
>  ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY))
>  export KDEVOPS_SSH_PRIVKEY:=$(basename $(KDEVOPS_SSH_PUBKEY))
> -TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_privkey_file='$(KDEVOPS_SSH_PRIVKEY)'
>  
>  ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY_OVERWRITE))
>  DEFAULT_DEPS += remove-ssh-key

I need to think about this one / test it. Might have side effects, but
maybe not.


-- 
Chuck Lever

Re: [PATCH 1/3] Set kdevops_terraform_ssh_config_privkey_file unconditionally

[Chuck Lever]

On 5/2/25 11:32 AM, Chuck Lever wrote:
> On 5/2/25 2:46 AM, Chandan Babu R wrote:
>> kdevops_terraform_ssh_config_privkey_file is not set on control systems which
>> use pre-existing ssh keys. Hence, adding entries of provisioned cloud
>> instances to the ephemeral ssh config file fails on such systems.
>>
>> Fix this by setting kdevops_terraform_ssh_config_privkey_file
>> unconditionally.
>>
>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>> ---
>>  scripts/terraform.Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/terraform.Makefile b/scripts/terraform.Makefile
>> index 91469469..b6b26e69 100644
>> --- a/scripts/terraform.Makefile
>> +++ b/scripts/terraform.Makefile
>> @@ -85,10 +85,10 @@ endif # CONFIG_KDEVOPS_SSH_CONFIG_UPDATE
>>  export KDEVOPS_SSH_PUBKEY:=$(shell realpath $(subst ",,$(CONFIG_TERRAFORM_SSH_CONFIG_PUBKEY_FILE)))
>>  TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_pubkey_file='$(KDEVOPS_SSH_PUBKEY)'
>>  TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_user='$(SSH_CONFIG_USER)'
>> +TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_privkey_file='$(basename $(KDEVOPS_SSH_PUBKEY))'
>>  
>>  ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY))
>>  export KDEVOPS_SSH_PRIVKEY:=$(basename $(KDEVOPS_SSH_PUBKEY))
>> -TERRAFORM_EXTRA_VARS += kdevops_terraform_ssh_config_privkey_file='$(KDEVOPS_SSH_PRIVKEY)'
>>  
>>  ifeq (y,$(CONFIG_TERRAFORM_SSH_CONFIG_GENKEY_OVERWRITE))
>>  DEFAULT_DEPS += remove-ssh-key
> 
> I need to think about this one / test it. Might have side effects, but
> maybe not.
> 
> 

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>

and confirmed it's doesn't seem to break anything in a simple smoke
test on Azure.


-- 
Chuck Lever

Re: [PATCH 3/3] Allow private IP address to be assigned to instances created on OCI

[Chuck Lever]

On 5/2/25 11:30 AM, Chuck Lever wrote:
> On 5/2/25 2:46 AM, Chandan Babu R wrote:
>> The OCI tenancy available to kernel developers from Oracle allows only private
>> IP addresses to be assigned to compute instances. Hence this commit brings
>> back the ability to work with OCI instances having private IP addresses.
>>
>> The variable "public_ip_map" defined in terraform/*/output.tf files has also
>> been changed to "ansible_ip_map" in order to abstract away the type of the IP
>> address being used.

Duh. I know I suggested the name, but "ansible_ip_map" might suggest
that Ansible defines this variable, like all other variables named
ansible_something.

Other possibilities:

 ips_for_ansible
 controller_ip_map



>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>> ---
>>
>> Please note that since I have only an OCI account, I haven't tested
>> the changes on other cloud providers. However, the changes are limited
>> to renaming "public_ip_map" to "ansible_ip_map" in terraform files.
>>
>>  playbooks/roles/terraform/tasks/main.yml          |  8 ++++----
>>  playbooks/roles/terraform/templates/ssh_config.j2 |  4 ++--
>>  scripts/status_terraform.sh                       |  2 +-
>>  terraform/aws/output.tf                           |  2 +-
>>  terraform/azure/output.tf                         |  2 +-
>>  terraform/gce/output.tf                           |  2 +-
>>  terraform/oci/output.tf                           | 14 +++++++++-----
>>  terraform/openstack/output.tf                     |  2 +-
>>  8 files changed, 20 insertions(+), 16 deletions(-)
>>
>> diff --git a/playbooks/roles/terraform/tasks/main.yml b/playbooks/roles/terraform/tasks/main.yml
>> index d04dc997..d1c94264 100644
>> --- a/playbooks/roles/terraform/tasks/main.yml
>> +++ b/playbooks/roles/terraform/tasks/main.yml
>> @@ -7,22 +7,22 @@
>>    tags:
>>      - bringup
>>  
>> -- name: Retrieve the public_ip_map from terraform
>> +- name: Retrieve the ansible_ip_map from terraform
>>    delegate_to: localhost
>>    run_once: true
>>    ansible.builtin.command:
>>      chdir: "{{ topdir_path }}/terraform/{{ kdevops_terraform_provider }}"
>> -    cmd: "terraform output -json public_ip_map"
>> +    cmd: "terraform output -json ansible_ip_map"
>>    register: terraform_output
>>    changed_when: false
>>    tags:
>>      - ssh
>>  
>> -- name: Convert the retrieved public_ip_map into a dictionary
>> +- name: Convert the retrieved ansible_ip_map into a dictionary
>>    delegate_to: localhost
>>    run_once: true
>>    ansible.builtin.set_fact:
>> -    public_ip_map: "{{ terraform_output.stdout | from_json }}"
>> +    ansible_ip_map: "{{ terraform_output.stdout | from_json }}"
>>    tags:
>>      - ssh
>>  
>> diff --git a/playbooks/roles/terraform/templates/ssh_config.j2 b/playbooks/roles/terraform/templates/ssh_config.j2
>> index d9b057b1..c6e62a61 100644
>> --- a/playbooks/roles/terraform/templates/ssh_config.j2
>> +++ b/playbooks/roles/terraform/templates/ssh_config.j2
>> @@ -1,5 +1,5 @@
>> -Host {{ inventory_hostname }} {{ public_ip_map[inventory_hostname] }}
>> -	HostName {{ public_ip_map[inventory_hostname] }}
>> +Host {{ inventory_hostname }} {{ ansible_ip_map[inventory_hostname] }}
>> +	HostName {{ ansible_ip_map[inventory_hostname] }}
>>  	User {{ kdevops_terraform_ssh_config_user }}
>>  	Port 22
>>  	IdentityFile {{ kdevops_terraform_ssh_config_privkey_file }}
>> diff --git a/scripts/status_terraform.sh b/scripts/status_terraform.sh
>> index dc82d79e..bc4ad309 100755
>> --- a/scripts/status_terraform.sh
>> +++ b/scripts/status_terraform.sh
>> @@ -13,5 +13,5 @@ if [[ ${COUNT} -eq 1 ]]; then
>>  else
>>    echo "There are ${COUNT} active terraform resources on '$1'."
>>  fi
>> -terraform output public_ip_map
>> +terraform output ansible_ip_map
>>  exit 0
>> diff --git a/terraform/aws/output.tf b/terraform/aws/output.tf
>> index ab18bc55..b5e59eb1 100644
>> --- a/terraform/aws/output.tf
>> +++ b/terraform/aws/output.tf
>> @@ -3,7 +3,7 @@
>>  # Each provider's output.tf needs to define a public_ip_map. This
>>  # map is used to build the Ansible controller's ssh configuration.
>>  # Each map entry contains the node's hostname and public IP address.
>> -output "public_ip_map" {
>> +output "ansible_ip_map" {
>>    description = "The public IP addresses assigned to each instance"
>>    value       = zipmap(var.kdevops_nodes[*], aws_eip.kdevops_eip[*].public_ip)
>>  }
>> diff --git a/terraform/azure/output.tf b/terraform/azure/output.tf
>> index 38104c90..deb7d141 100644
>> --- a/terraform/azure/output.tf
>> +++ b/terraform/azure/output.tf
>> @@ -1,7 +1,7 @@
>>  # Each provider's output.tf needs to define a public_ip_map. This
>>  # map is used to build the Ansible controller's ssh configuration.
>>  # Each map entry contains the node's hostname and public IP address.
>> -output "public_ip_map" {
>> +output "ansible_ip_map" {
>>    description = "The public IP addresses assigned to each instance"
>>    value       = zipmap(var.kdevops_nodes[*], azurerm_public_ip.kdevops_publicip[*].ip_address)
>>  }
>> diff --git a/terraform/gce/output.tf b/terraform/gce/output.tf
>> index 15889cc4..9246216f 100644
>> --- a/terraform/gce/output.tf
>> +++ b/terraform/gce/output.tf
>> @@ -3,7 +3,7 @@
>>  # Each provider's output.tf needs to define a public_ip_map. This
>>  # map is used to build the Ansible controller's ssh configuration.
>>  # Each map entry contains the node's hostname and public IP address.
>> -output "public_ip_map" {
>> +output "ansible_ip_map" {
>>    description = "The public IP addresses assigned to each instance"
>>    value = zipmap(var.kdevops_nodes[*],
>>    google_compute_instance.kdevops_instance[*].network_interface[0].access_config[0].nat_ip)
>> diff --git a/terraform/oci/output.tf b/terraform/oci/output.tf
>> index 1657b804..6562a1e8 100644
>> --- a/terraform/oci/output.tf
>> +++ b/terraform/oci/output.tf
>> @@ -1,9 +1,13 @@
>>  # All generic output goes here
>>  
>> -# Each provider's output.tf needs to define a public_ip_map. This
>> +# Each provider's output.tf needs to define a ansible_ip_map. This
>>  # map is used to build the Ansible controller's ssh configuration.
>> -# Each map entry contains the node's hostname and public IP address.
>> -output "public_ip_map" {
>> -  description = "The public IP addresses assigned to each instance"
>> -  value       = zipmap(var.kdevops_nodes[*], oci_core_instance.kdevops_instance.*.public_ip)
>> +# Each map entry contains the node's hostname and public/private IP
>> +# address.
> 
> Can you update the documenting comment in the output.tf file for the
> other providers as well?
> 
> 
>> +output "ansible_ip_map" {
>> +  description = "The IP addresses assigned to each instance"
> 
> Also, update the description for the ansible_ip_map output variable in
> all the output.tf files.
> 
> 
>> +  value       = zipmap(var.kdevops_nodes[*],
>> +    var.oci_assign_public_ip ?
>> +    oci_core_instance.kdevops_instance.*.public_ip :
>> +    oci_core_instance.kdevops_instance.*.private_ip)
> 
> Weird indentation. I assume this was the result of "terraform fmt" ?
> 
> 
>>  }
>> diff --git a/terraform/openstack/output.tf b/terraform/openstack/output.tf
>> index d5d3a6b1..3dd139a3 100644
>> --- a/terraform/openstack/output.tf
>> +++ b/terraform/openstack/output.tf
>> @@ -21,7 +21,7 @@ output "kdevops_hosts_and_ipv4" {
>>  # Each provider's output.tf needs to define a public_ip_map. This
>>  # map is used to build the Ansible controller's ssh configuration.
>>  # Each map entry contains the node's hostname and public IP address.
>> -output "public_ip_map" {
>> +output "ansible_ip_map" {
>>    description = "The public IP addresses assigned to each instance"
>>    value       = zipmap(var.kdevops_nodes[*], openstack_compute_instance_v2.kdevops_instances[*].access_ip_v4)
>>  }
> 
> This patch makes sense to me, just needs a little polish.
> 
> 


-- 
Chuck Lever

Re: [PATCH 3/3] Allow private IP address to be assigned to instances created on OCI

[Chandan Babu R]

On Fri, May 02, 2025 at 12:31:58 PM -0400, Chuck Lever wrote:
> On 5/2/25 11:30 AM, Chuck Lever wrote:
>> On 5/2/25 2:46 AM, Chandan Babu R wrote:
>>> The OCI tenancy available to kernel developers from Oracle allows only private
>>> IP addresses to be assigned to compute instances. Hence this commit brings
>>> back the ability to work with OCI instances having private IP addresses.
>>>
>>> The variable "public_ip_map" defined in terraform/*/output.tf files has also
>>> been changed to "ansible_ip_map" in order to abstract away the type of the IP
>>> address being used.
>
> Duh. I know I suggested the name, but "ansible_ip_map" might suggest
> that Ansible defines this variable, like all other variables named
> ansible_something.
>
> Other possibilities:
>
>  ips_for_ansible
>  controller_ip_map
>

I think controller_ip_map is a better name. I will use that.

>
>
>>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>>> ---
>>>
>>> Please note that since I have only an OCI account, I haven't tested
>>> the changes on other cloud providers. However, the changes are limited
>>> to renaming "public_ip_map" to "ansible_ip_map" in terraform files.
>>>
>>>  playbooks/roles/terraform/tasks/main.yml          |  8 ++++----
>>>  playbooks/roles/terraform/templates/ssh_config.j2 |  4 ++--
>>>  scripts/status_terraform.sh                       |  2 +-
>>>  terraform/aws/output.tf                           |  2 +-
>>>  terraform/azure/output.tf                         |  2 +-
>>>  terraform/gce/output.tf                           |  2 +-
>>>  terraform/oci/output.tf                           | 14 +++++++++-----
>>>  terraform/openstack/output.tf                     |  2 +-
>>>  8 files changed, 20 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/playbooks/roles/terraform/tasks/main.yml b/playbooks/roles/terraform/tasks/main.yml
>>> index d04dc997..d1c94264 100644
>>> --- a/playbooks/roles/terraform/tasks/main.yml
>>> +++ b/playbooks/roles/terraform/tasks/main.yml
>>> @@ -7,22 +7,22 @@
>>>    tags:
>>>      - bringup
>>>  
>>> -- name: Retrieve the public_ip_map from terraform
>>> +- name: Retrieve the ansible_ip_map from terraform
>>>    delegate_to: localhost
>>>    run_once: true
>>>    ansible.builtin.command:
>>>      chdir: "{{ topdir_path }}/terraform/{{ kdevops_terraform_provider }}"
>>> -    cmd: "terraform output -json public_ip_map"
>>> +    cmd: "terraform output -json ansible_ip_map"
>>>    register: terraform_output
>>>    changed_when: false
>>>    tags:
>>>      - ssh
>>>  
>>> -- name: Convert the retrieved public_ip_map into a dictionary
>>> +- name: Convert the retrieved ansible_ip_map into a dictionary
>>>    delegate_to: localhost
>>>    run_once: true
>>>    ansible.builtin.set_fact:
>>> -    public_ip_map: "{{ terraform_output.stdout | from_json }}"
>>> +    ansible_ip_map: "{{ terraform_output.stdout | from_json }}"
>>>    tags:
>>>      - ssh
>>>  
>>> diff --git a/playbooks/roles/terraform/templates/ssh_config.j2 b/playbooks/roles/terraform/templates/ssh_config.j2
>>> index d9b057b1..c6e62a61 100644
>>> --- a/playbooks/roles/terraform/templates/ssh_config.j2
>>> +++ b/playbooks/roles/terraform/templates/ssh_config.j2
>>> @@ -1,5 +1,5 @@
>>> -Host {{ inventory_hostname }} {{ public_ip_map[inventory_hostname] }}
>>> -	HostName {{ public_ip_map[inventory_hostname] }}
>>> +Host {{ inventory_hostname }} {{ ansible_ip_map[inventory_hostname] }}
>>> +	HostName {{ ansible_ip_map[inventory_hostname] }}
>>>  	User {{ kdevops_terraform_ssh_config_user }}
>>>  	Port 22
>>>  	IdentityFile {{ kdevops_terraform_ssh_config_privkey_file }}
>>> diff --git a/scripts/status_terraform.sh b/scripts/status_terraform.sh
>>> index dc82d79e..bc4ad309 100755
>>> --- a/scripts/status_terraform.sh
>>> +++ b/scripts/status_terraform.sh
>>> @@ -13,5 +13,5 @@ if [[ ${COUNT} -eq 1 ]]; then
>>>  else
>>>    echo "There are ${COUNT} active terraform resources on '$1'."
>>>  fi
>>> -terraform output public_ip_map
>>> +terraform output ansible_ip_map
>>>  exit 0
>>> diff --git a/terraform/aws/output.tf b/terraform/aws/output.tf
>>> index ab18bc55..b5e59eb1 100644
>>> --- a/terraform/aws/output.tf
>>> +++ b/terraform/aws/output.tf
>>> @@ -3,7 +3,7 @@
>>>  # Each provider's output.tf needs to define a public_ip_map. This
>>>  # map is used to build the Ansible controller's ssh configuration.
>>>  # Each map entry contains the node's hostname and public IP address.
>>> -output "public_ip_map" {
>>> +output "ansible_ip_map" {
>>>    description = "The public IP addresses assigned to each instance"
>>>    value       = zipmap(var.kdevops_nodes[*], aws_eip.kdevops_eip[*].public_ip)
>>>  }
>>> diff --git a/terraform/azure/output.tf b/terraform/azure/output.tf
>>> index 38104c90..deb7d141 100644
>>> --- a/terraform/azure/output.tf
>>> +++ b/terraform/azure/output.tf
>>> @@ -1,7 +1,7 @@
>>>  # Each provider's output.tf needs to define a public_ip_map. This
>>>  # map is used to build the Ansible controller's ssh configuration.
>>>  # Each map entry contains the node's hostname and public IP address.
>>> -output "public_ip_map" {
>>> +output "ansible_ip_map" {
>>>    description = "The public IP addresses assigned to each instance"
>>>    value       = zipmap(var.kdevops_nodes[*], azurerm_public_ip.kdevops_publicip[*].ip_address)
>>>  }
>>> diff --git a/terraform/gce/output.tf b/terraform/gce/output.tf
>>> index 15889cc4..9246216f 100644
>>> --- a/terraform/gce/output.tf
>>> +++ b/terraform/gce/output.tf
>>> @@ -3,7 +3,7 @@
>>>  # Each provider's output.tf needs to define a public_ip_map. This
>>>  # map is used to build the Ansible controller's ssh configuration.
>>>  # Each map entry contains the node's hostname and public IP address.
>>> -output "public_ip_map" {
>>> +output "ansible_ip_map" {
>>>    description = "The public IP addresses assigned to each instance"
>>>    value = zipmap(var.kdevops_nodes[*],
>>>    google_compute_instance.kdevops_instance[*].network_interface[0].access_config[0].nat_ip)
>>> diff --git a/terraform/oci/output.tf b/terraform/oci/output.tf
>>> index 1657b804..6562a1e8 100644
>>> --- a/terraform/oci/output.tf
>>> +++ b/terraform/oci/output.tf
>>> @@ -1,9 +1,13 @@
>>>  # All generic output goes here
>>>  
>>> -# Each provider's output.tf needs to define a public_ip_map. This
>>> +# Each provider's output.tf needs to define a ansible_ip_map. This
>>>  # map is used to build the Ansible controller's ssh configuration.
>>> -# Each map entry contains the node's hostname and public IP address.
>>> -output "public_ip_map" {
>>> -  description = "The public IP addresses assigned to each instance"
>>> -  value       = zipmap(var.kdevops_nodes[*], oci_core_instance.kdevops_instance.*.public_ip)
>>> +# Each map entry contains the node's hostname and public/private IP
>>> +# address.
>> 
>> Can you update the documenting comment in the output.tf file for the
>> other providers as well?
>> 
>>

Ok.

>>> +output "ansible_ip_map" {
>>> +  description = "The IP addresses assigned to each instance"
>> 
>> Also, update the description for the ansible_ip_map output variable in
>> all the output.tf files.
>>

Sure, I will do that.

>> 
>>> +  value       = zipmap(var.kdevops_nodes[*],
>>> +    var.oci_assign_public_ip ?
>>> +    oci_core_instance.kdevops_instance.*.public_ip :
>>> +    oci_core_instance.kdevops_instance.*.private_ip)
>> 
>> Weird indentation. I assume this was the result of "terraform fmt" ?

Sorry, I had used my text editor's default indentation. I will fix it.

>> 
>> 
>>>  }
>>> diff --git a/terraform/openstack/output.tf b/terraform/openstack/output.tf
>>> index d5d3a6b1..3dd139a3 100644
>>> --- a/terraform/openstack/output.tf
>>> +++ b/terraform/openstack/output.tf
>>> @@ -21,7 +21,7 @@ output "kdevops_hosts_and_ipv4" {
>>>  # Each provider's output.tf needs to define a public_ip_map. This
>>>  # map is used to build the Ansible controller's ssh configuration.
>>>  # Each map entry contains the node's hostname and public IP address.
>>> -output "public_ip_map" {
>>> +output "ansible_ip_map" {
>>>    description = "The public IP addresses assigned to each instance"
>>>    value       = zipmap(var.kdevops_nodes[*], openstack_compute_instance_v2.kdevops_instances[*].access_ip_v4)
>>>  }
>> 
>> This patch makes sense to me, just needs a little polish.
>> 

Thanks for reviewing all the patches.

I will make the required modifications and post V2 soon.

-- 
Chandan