address comments

Author: mainred

src/aks-agent/README.rst

@@ -105,7 +105,7 @@ Run in non-interactive batch mode
 
 .. code-block:: bash
 
-    az aks agent "Diagnose networking issues" --no-interactive --max-steps 15 --model azure/my-gpt4.1-deployment
+    az aks agent "Diagnose networking issues" --no-interactive --name MyManagedCluster --resource-group MyResourceGroup --model azure/my-gpt4.1-deployment
 
 Clean up the AKS agent
 -----------------------

src/aks-agent/azext_aks_agent/_help.py

@@ -56,7 +56,7 @@
         - name: Ask about pod issues in the cluster with last configured model
           text: |-
             az aks agent "Why are my pods not starting?" --resource-group myResourceGroup --name myAKSCluster
-          - name: Check AKS agent deployment status
+        - name: Check AKS agent deployment status
           text: |-
             az aks agent --status --resource-group myResourceGroup --name myAKSCluster
         - name: Ask about pod issues in the cluster with Azure OpenAI

src/aks-agent/azext_aks_agent/agent/aks.py

@@ -39,7 +39,7 @@ def get_aks_credentials(
     kubeconfig_path = _get_kubeconfig_file_path(resource_group_name, cluster_name)
 
     # Ensure the kubeconfig file exists and write kubeconfig to it
-    with os.fdopen(os.open(kubeconfig_path, os.O_CREAT | os.O_WRONLY, 0o600), 'wt') as f:
+    with os.fdopen(os.open(kubeconfig_path, os.O_RDWR | os.O_CREAT | os.O_TRUNC, 0o600), 'wt') as f:
         f.write(kubeconfig)
 
     logger.info("Kubeconfig downloaded successfully to: %s", kubeconfig_path)
@@ -64,7 +64,7 @@ def _get_kubeconfig_file_path(  # pylint: disable=unused-argument
             if ex.errno != errno.EEXIST:
                 raise
 
-    kubeconfig_filename = f"kubeconfig-{cluster_name}"
+    kubeconfig_filename = f"kubeconfig-{resource_group_name}-{cluster_name}"
     kubeconfig_path = os.path.join(kubeconfig_dir, kubeconfig_filename)
 
     return kubeconfig_path

src/aks-agent/azext_aks_agent/custom.py

@@ -174,7 +174,7 @@ def aks_agent_init(cmd,
 
     except Exception as e:
         console.print(f"❌ Error during initialization: {str(e)}", style=ERROR_COLOR)
-        logger.error("Agent initialization failed: %s", e)
+        raise AzCLIError(f"Agent initialization failed: {str(e)}")
 
 
 def _get_existing_cluster_role(aks_agent_manager):
@@ -431,10 +431,9 @@ def aks_agent_cleanup(
         console.print(
             "❌ Cleanup failed. Please run 'az aks agent --status' to verify cleanup completion.", style=ERROR_COLOR)
 
+
 # pylint: disable=unused-argument
 # pylint: disable=too-many-locals
-
-
 def aks_agent(
     cmd,
     client,

src/aks-agent/azext_aks_agent/tests/latest/test_custom_init.py

@@ -73,15 +73,16 @@ def test_init_new_deployment_no_llm_config(
         mock_prompt_managed_identity.assert_called_once()
         mock_agent_manager.deploy_agent.assert_called_once()
 
+    @patch('azext_aks_agent.custom._get_existing_cluster_role')
     @patch('azext_aks_agent.custom._setup_and_create_llm_config')
     @patch('azext_aks_agent.custom.get_aks_credentials')
     @patch('azext_aks_agent.custom.AKSAgentManager')
     @patch('azext_aks_agent.custom.get_subscription_id')
     @patch('azext_aks_agent.custom.get_console')
     def test_init_existing_llm_config_user_skips_update(
             self, mock_get_console, mock_get_subscription_id, mock_aks_manager_class,
-            mock_get_aks_creds, mock_setup_llm):
-        """Test initialization when LLM config exists and user skips update."""
+            mock_get_aks_creds, mock_setup_llm, mock_get_cluster_role):
+        """Test initialization raises AzCLIError when LLM config exists, user skips update, but cluster role not found."""
         # Setup mocks
         mock_console = MagicMock()
         mock_get_console.return_value = mock_console
@@ -98,22 +99,29 @@ def test_init_existing_llm_config_user_skips_update(
         mock_agent_manager.managed_identity_client_id = ""
         mock_aks_manager_class.return_value = mock_agent_manager
 
-        # Execute
-        aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
+        mock_get_cluster_role.return_value = None  # Cannot find cluster role
+
+        # Execute - should raise AzCLIError with wrapped message
+        with self.assertRaises(AzCLIError) as cm:
+            aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
 
         # Assert
         mock_agent_manager.check_llm_config_exists.assert_called_once()
         mock_setup_llm.assert_not_called()
+        # Verify the error message contains the wrapped format
+        self.assertIn("Agent initialization failed:", str(cm.exception))
+        self.assertIn("Could not determine existing cluster role", str(cm.exception))
 
+    @patch('azext_aks_agent.custom._get_existing_cluster_role')
     @patch('azext_aks_agent.custom._setup_and_create_llm_config')
     @patch('azext_aks_agent.custom.get_aks_credentials')
     @patch('azext_aks_agent.custom.AKSAgentManager')
     @patch('azext_aks_agent.custom.get_subscription_id')
     @patch('azext_aks_agent.custom.get_console')
     def test_init_existing_llm_config_user_updates(
             self, mock_get_console, mock_get_subscription_id, mock_aks_manager_class,
-            mock_get_aks_creds, mock_setup_llm):
-        """Test initialization when LLM config exists and user chooses to update."""
+            mock_get_aks_creds, mock_setup_llm, mock_get_cluster_role):
+        """Test initialization raises AzCLIError when LLM config exists, user updates, but cluster role not found."""
         # Setup mocks
         mock_console = MagicMock()
         mock_get_console.return_value = mock_console
@@ -130,12 +138,18 @@ def test_init_existing_llm_config_user_updates(
         mock_agent_manager.managed_identity_client_id = ""
         mock_aks_manager_class.return_value = mock_agent_manager
 
-        # Execute
-        aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
+        mock_get_cluster_role.return_value = None  # Cannot find cluster role
+
+        # Execute - should raise AzCLIError with wrapped message
+        with self.assertRaises(AzCLIError) as cm:
+            aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
 
         # Assert
         mock_agent_manager.check_llm_config_exists.assert_called_once()
         mock_setup_llm.assert_called_once_with(mock_console, mock_agent_manager)
+        # Verify the error message contains the wrapped format
+        self.assertIn("Agent initialization failed:", str(cm.exception))
+        self.assertIn("Could not determine existing cluster role", str(cm.exception))
 
     @patch('azext_aks_agent.custom._get_existing_cluster_role')
     @patch('azext_aks_agent.custom._display_cluster_role_rules')
@@ -226,7 +240,7 @@ def test_init_with_custom_cluster_role(
     def test_init_deployment_failure_logs_error(
             self, mock_get_console, mock_get_subscription_id, mock_aks_manager_class,
             mock_get_aks_creds, mock_setup_llm):
-        """Test initialization logs error when deployment fails."""
+        """Test initialization raises AzCLIError when deployment fails."""
         # Setup mocks
         mock_console = MagicMock()
         mock_get_console.return_value = mock_console
@@ -239,13 +253,13 @@ def test_init_deployment_failure_logs_error(
         mock_agent_manager.deploy_agent.return_value = (False, "Deployment failed")
         mock_aks_manager_class.return_value = mock_agent_manager
 
-        # Execute - should not raise, but handle the error internally
-        aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
+        # Execute - should raise AzCLIError with wrapped message
+        with self.assertRaises(AzCLIError) as cm:
+            aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
 
-        # Verify error message was printed to console
-        error_calls = [call for call in mock_console.print.call_args_list
-                       if 'Failed to deploy agent' in str(call)]
-        self.assertTrue(len(error_calls) > 0, "Expected error message about deployment failure to be printed")
+        # Verify the error message contains the wrapped format
+        self.assertIn("Agent initialization failed:", str(cm.exception))
+        self.assertIn("Failed to deploy agent", str(cm.exception))
 
     @patch('azext_aks_agent.custom._get_existing_cluster_role')
     @patch('azext_aks_agent.custom._setup_and_create_llm_config')
@@ -256,7 +270,7 @@ def test_init_deployment_failure_logs_error(
     def test_init_deployed_no_cluster_role_logs_error(
             self, mock_get_console, mock_get_subscription_id, mock_aks_manager_class,
             mock_get_aks_creds, mock_setup_llm, mock_get_cluster_role):
-        """Test initialization logs error when deployed but cannot determine cluster role."""
+        """Test initialization raises AzCLIError when deployed but cannot determine cluster role."""
         # Setup mocks
         mock_console = MagicMock()
         mock_get_console.return_value = mock_console
@@ -271,13 +285,13 @@ def test_init_deployed_no_cluster_role_logs_error(
 
         mock_get_cluster_role.return_value = None  # Cannot find cluster role
 
-        # Execute - should not raise, but handle the error internally
-        aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
+        # Execute - should raise AzCLIError with wrapped message
+        with self.assertRaises(AzCLIError) as cm:
+            aks_agent_init(self.mock_cmd, self.mock_client, self.resource_group, self.cluster_name)
 
-        # Verify error message was printed to console
-        error_calls = [call for call in mock_console.print.call_args_list
-                       if 'Could not determine existing cluster role' in str(call)]
-        self.assertTrue(len(error_calls) > 0, "Expected error message about cluster role to be printed")
+        # Verify the error message contains the wrapped format
+        self.assertIn("Agent initialization failed:", str(cm.exception))
+        self.assertIn("Could not determine existing cluster role", str(cm.exception))
 
     @patch('azext_aks_agent.custom._prompt_managed_identity_configuration')
     @patch('azext_aks_agent.custom._prompt_cluster_role_configuration')