azure_rm_aks: support system-assigned (managed) identity,

[geekq]

... make linux_profile and service_principal optional

SUMMARY

Motivation:

while current azure documentation tells

The cluster infrastructure authentication specified is used by Azure Kubernetes Service to manage cloud resources attached to the cluster. This can be either a service principal or a system-assigned managed identity.

the current azcollection only supports former, not latter

azure/plugins/modules/azure_rm_aks.py

Line 121 in 9e20c6e

required: true

declares service_principal as required and allows no system-assigned managed identity.

An explicit linux_profile, service_principal require additional work at the beginning, but especially later, for rotation credentials for the service principal. The usual (and also Microsoft-recommended) approach nowadays is to use system-assigned managed identity. Also added an example for creating a cluster using minimal number of parameters.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

azure_rm_aks

ADDITIONAL INFORMATION

If you just start with trying to automate the Azure k8s cluster creation with ansible,
it was surprising to see many parameters required by this ansible module.

While az cli only requires name, group name and node count
az aks create -g myGroup -n myCluster --node-count 3 -l westeurope - you just need an existing resource group,
I saw no way to create a cluster with a single azure_rm_aks ansible module call.

This pull request aims to change that, enabling an easy start, making following example work

    - name: Use minimal parameters and system-assigned identity
      azure_rm_aks:
        name: myMinimalCluster
        location: eastus
        resource_group: myExistingResourceGroup
        dns_prefix: akstest
        agent_pool_profiles:
          - name: default
            count: 1
            vm_size: Standard_D2_v2

[Fred-sun]

@geekq Thank for your contribution! Would you add test case for the request? Thank you very much!

[Fred-sun]

The required is not match with argument_spec. The argument_spec is required!

[geekq]

I was unsure, if I should remove required: true for the suboptions too. Current state of PR seems to work as expected. I understand it so: linux_profile is optional, but if linux_profile: is given, then suboptions admin_username and ssh_key must be provided. This is why I kept required: true for admin_username and ssh_key. Or do you mean something else with "The argument_spec is required"?

[geekq]

Would you add test case for the request?

Would love to provide tests! From CONTRIBUTING.md or README.md I did not understand, what tests are needed, how they are supposed to be run: do I have to run the whole suit against a real Azure account? Maybe you can provide some guidance on this, thanks! Otherwise I'll try to dive in into the code and may be extend https://github.com/ansible-collections/azure/blob/dev/tests/integration/targets/azure_rm_aks/tasks/main.yml - or can I (preferably) create a smaller, isolated test in the same folder?

P.S. Can I see the test results in the CI? Looks like it requires additional authorization?

[geekq]

I've added an integration test for this minimal-parameters-cluster-definition.
But how I can run it with tests/utils/ado/ado.sh locally?

Or maybe @Fred-sun you can see the test results in the CI (I seem to have no access to the linked pipelines above)

[Xiuxi-Sun]

I've added an integration test for this minimal-parameters-cluster-definition.
But how I can run it with tests/utils/ado/ado.sh locally?

Or maybe @Fred-sun you can see the test results in the CI (I seem to have no access to the linked pipelines above)

@geekq You can follow the link below to perform the Sanity test. Thank you very much!

link:  https://docs.ansible.com/ansible/latest/dev_guide/testing_sanity.html
command:   ansible-test sanity --color -v --junit

[Fred-sun]

@geekq Why don't you place the test（minimal-cluster.yml） in main.yml so that the automated test can cover the feature! Thank you very much!

[Fred-sun]

@geekq In addition, please help to share the results of the test cases you added, which will help to advance the PR merge. Thank you very much!

[Fred-sun]

Please add "kubernetes_version" parameter when create kubernets service! Thank you very much!

Suggested change

	resource_group: "{{ resource_group }}"
	resource_group: "{{ resource_group }}"
	kubernetes_version: **

[geekq]

@geekq Why don't you place the test（minimal-cluster.yml in main.yml

My motivation for putting it to a separate file was to depict, that those tasks are self-contained and do not depend on the other steps in main.yml
Such separation in smaller, self contained chunks would improve the maintenance. So anybody working on a particular feature could run just a single file (much faster, costs less azure resource). I would even recommend to break the existing main.yml into small, independent chunks! But I do not mind, if you merge it to the big main.yml ;-)

so that the automated test can cover the feature!

Maybe this automated test could run multiple files. It would be also great to be able to see it's output published somewhere.

Please add "kubernetes_version" parameter when create kubernets service!

What about maintenance? azure constantly changes the list of kubernetes versions available. You will need to adjust this file continuously. But I do not mind! :-) Please adjust as you see fit. It is maybe less work for you to adjust this PR to your taste, than make me change each line ;-) And thank you for maintaining this library!

[Fred-sun]

@geekq Thank you for your reply.

First, kubernetes_version is a required parameter when creating aks, and we can get the version of kubernetes_version from azure_rm_aksversion_info and pass it in.

Second, the main.yml is an automated playbook. Test /// Tasks /*.yml will be executed on a weekly basis to ensure that the latest repo is working properly.

In the end, we combined the two PlayBooks. It doesn't have to be interdependent, it can be a separate test module, with different functions for the test.

[Fred-sun]

@geekq Could you please help to authorize me to update this PR? Thank you very much!

[Fred-sun]

@geekq Please make the final changes. Thank you very much!

File: tests/integration/targets/azure_rm_aks/tasks/main.yml
 - set_fact:
     rpfx: "{{ resource_group | hash('md5') | truncate(8, True, '') }}"
     noderpfx: "{{ resource_group | hash('md5') | truncate(4, True, '') }}"

 - include: minimal-cluster.yml

 - name: Find available k8s version
   azure_rm_aksversion_info:
     location: eastus
   register: versions

File :tests/integration/targets/azure_rm_aks/tasks/minimal-cluster.yml
- set_fact:
    rpfx: "{{ resource_group | hash('md5') | truncate(8, True, '') }}"
    noderpfx: "{{ resource_group | hash('md5') | truncate(4, True, '') }}"

- name: Find available k8s version
  azure_rm_aksversion_info:
    location: eastus
  register: versions

- name: Use minimal parameters and system-assigned identity
  azure_rm_aks:
    name: "minimal{{ rpfx }}"
    location: eastus
    resource_group: "{{ resource_group }}"
    dns_prefix: "aks{{ rpfx }}"
    kubernetes_version: "{{ versions.azure_aks_versions[0] }}"
    agent_pool_profiles:
      - name: default
        count: 1
        vm_size: Standard_DS1_v2
  register: output

- name: Assert the AKS instance is well created
  assert:
    that:
      - output.changed
      - output.provisioning_state == 'Succeeded'

- name: Get AKS fact
  azure_rm_aks_info:
     name: "minimal{{ rpfx }}"
     resource_group: "{{ resource_group }}"
  register: fact

- name: Assert fact returns the created one
  assert:
    that:
      - "fact.aks | length == 1"
      - fact.aks[0].id == output.id

- name: Use minimal parameters and system-assigned identity (idempotent)
  azure_rm_aks:
    name: "minimal{{ rpfx }}"
    location: eastus
    resource_group: "{{ resource_group }}"
    dns_prefix: "aks{{ rpfx }}"
    kubernetes_version: "{{ versions.azure_aks_versions[0] }}"
    agent_pool_profiles:
      - name: default
        count: 1
        vm_size: Standard_DS1_v2
  register: output

- name: Assert idempotent
  assert:
    that:
      - not output.changed

- name: Delete the AKS instance
  azure_rm_aks:
    name: "minimal{{ rpfx }}"
    resource_group: "{{ resource_group }}"
    state: absent
  register: output

- name: Assert the AKS instance is well deleted
  assert:
    that:
      - output.changed

- name: Get AKS fact
  azure_rm_aks_info:
    name: "minimal{{ rpfx }}"
    resource_group: "{{ resource_group }}"
  register: fact

- name: Assert fact returns empty
  assert:
    that:
      - "fact.aks | length == 0"