Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

assign scaleset to a role #185

Merged
merged 23 commits into from
Oct 28, 2020
Merged

Conversation

chkeita
Copy link
Contributor

@chkeita chkeita commented Oct 21, 2020

Summary of the Pull Request

Allows the nodes in the scaleset to access the service by assigning their managed identity to the ManagedNode Role

PR Checklist

  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

What does this include?

Validation Steps Performed

  • Enable mandatory role assignment in your deployment
  • Create a scaleset
  • Execute the following `python src\deployment\registration.py assign_scaleset_role <onefuzz_instance_name> <scaleset_name>
  • Run a job on the and observe that it runs successfully

@chkeita chkeita marked this pull request as ready for review October 22, 2020 20:01
@chkeita chkeita marked this pull request as draft October 22, 2020 20:01
@chkeita chkeita marked this pull request as ready for review October 22, 2020 20:23
fix  app role update query
Copy link
Contributor

@bmc-msft bmc-msft left a comment

Choose a reason for hiding this comment

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

deploy.py uses CLIError to catch errors from add_application_password, which comes from azure-cli-core, which was removed as a prerequisite.

I'm guessing errors in add_application_password are no longer raised CLI errors and we should catch a different exception here, rather than re-adding azure-cli-core but I'm unsure of what that would be.

from azure.cli.core import CLIError

@bmc-msft bmc-msft dismissed their stale review October 26, 2020 14:41

Removing CLIError is addressed

Copy link
Contributor

@bmc-msft bmc-msft left a comment

Choose a reason for hiding this comment

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

Testing failed using clean venv:

Traceback (most recent call last):
  File "deploy.py", line 36, in <module>
    from azure.mgmt.applicationinsights import ApplicationInsightsManagementClient
ModuleNotFoundError: No module named 'azure.mgmt.applicationinsights'

Apparently, this module was pulled in via one of the prereq libraries that this PR removes.

@chkeita chkeita requested a review from bmc-msft October 27, 2020 05:20
@bmc-msft
Copy link
Contributor

In a fresh venv, this gives the following errors:

ERROR: azure-identity 1.4.1 has requirement msal<2.0.0,>=1.3.0, but you'll have msal 1.0.0 which is incompatible.
ERROR: azure-identity 1.4.1 has requirement msal-extensions~=0.2.2, but you'll have msal-extensions 0.1.3 which is incompatible.

@bmc-msft
Copy link
Contributor

Traceback (most recent call last):
  File "/tmp/tmpejuzjktw/deploy-venv/lib/python3.8/site-packages/azure/common/cloud.py", line 18, in get_cli_active_cloud
    from azure.cli.core.cloud import get_active_cloud
ModuleNotFoundError: No module named 'azure.cli'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "deploy.py", line 797, in <module>
    main()
  File "deploy.py", line 791, in main
    state[1](client)
  File "deploy.py", line 176, in check_region
    location = self.get_location_display_name()
  File "deploy.py", line 162, in get_location_display_name
    location_client = get_client_from_cli_profile(SubscriptionClient)
  File "/tmp/tmpejuzjktw/deploy-venv/lib/python3.8/site-packages/azure/common/client_factory.py", line 75, in get_client_from_cli_profile
    cloud = get_cli_active_cloud()
  File "/tmp/tmpejuzjktw/deploy-venv/lib/python3.8/site-packages/azure/common/cloud.py", line 20, in get_cli_active_cloud
    raise ImportError("You need to install 'azure-cli-core' to load CLI active Cloud")
ImportError: You need to install 'azure-cli-core' to load CLI active Cloud

restoring azure-cli and azure-cli-core
getting the token from cli-profile
@bmc-msft
Copy link
Contributor

Testing latest update

@bmc-msft bmc-msft merged commit e76064b into microsoft:main Oct 28, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Apr 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants