Pk5/add image parsing

src/aosm/HISTORY.rst

@@ -8,6 +8,8 @@ Unreleased
 * Add `publisher` command group for management of publisher resources.
 * Changed the name of the `path_to_mappings` parameter in the CNF input file to `default_values`
 * Added a `helm template` validation step to the `az aosm nfd build` command for the `cnf` definition type
+* Fixed helm chart image parsing in the `az aosm nfd build` command for the `cnf` definition type. This means that the images can now be extracted correctly from the helm chart.
+* Fixed: infinite loop bug when retrying failed artifact uploads to the ACR
 
 1.0.0b4
 ++++++++

src/aosm/azext_aosm/build_processors/helm_chart_processor.py

@@ -10,16 +10,26 @@
 from knack.log import get_logger
 
 from azext_aosm.build_processors.base_processor import BaseInputProcessor
-from azext_aosm.common.artifact import (BaseArtifact, LocalFileACRArtifact,
-                                        RemoteACRArtifact)
+from azext_aosm.common.artifact import (
+    BaseArtifact,
+    LocalFileACRArtifact,
+    RemoteACRArtifact,
+)
 from azext_aosm.common.local_file_builder import LocalFileBuilder
 from azext_aosm.inputs.helm_chart_input import HelmChartInput
 from azext_aosm.vendored_sdks.models import (
-    ApplicationEnablement, ArtifactType, AzureArcKubernetesArtifactProfile,
+    ApplicationEnablement,
+    ArtifactType,
+    AzureArcKubernetesArtifactProfile,
     AzureArcKubernetesDeployMappingRuleProfile,
-    AzureArcKubernetesHelmApplication, DependsOnProfile, HelmArtifactProfile,
-    HelmMappingRuleProfile, ManifestArtifactFormat, ReferencedResource,
-    ResourceElementTemplate)
+    AzureArcKubernetesHelmApplication,
+    DependsOnProfile,
+    HelmArtifactProfile,
+    HelmMappingRuleProfile,
+    ManifestArtifactFormat,
+    ReferencedResource,
+    ResourceElementTemplate,
+)
 
 logger = get_logger(__name__)
 
@@ -150,24 +160,25 @@ def generate_nf_application(self) -> AzureArcKubernetesHelmApplication:
         return AzureArcKubernetesHelmApplication(
             name=self.name,
             # Current implementation is set all depends on profiles to empty lists
-            depends_on_profile=DependsOnProfile(install_depends_on=[],
-                                                uninstall_depends_on=[], update_depends_on=[]),
+            depends_on_profile=DependsOnProfile(
+                install_depends_on=[], uninstall_depends_on=[], update_depends_on=[]
+            ),
             artifact_profile=artifact_profile,
             deploy_parameters_mapping_rule_profile=mapping_rule_profile,
         )
 
-    def _find_chart_images(self) -> List[Tuple[str, str]]:
+    def _find_chart_images(self) -> Set[Tuple[str, str]]:
         """
         Find the images used by the Helm chart.
 
         :return: A list of tuples containing the image name and version.
-        :rtype: List[Tuple[str, str]]
+        :rtype: Set[Tuple[str, str]]
         """
         logger.debug("Finding images used by Helm chart %s", self.name)
         image_lines: Set[str] = set()
         self._find_image_lines(self.input_artifact, image_lines)
 
-        images: List[Tuple[str, str]] = []
+        images: Set[Tuple[str, str]] = set()
         for line in image_lines:
             name_and_tag = re.search(IMAGE_NAME_AND_VERSION_REGEX, line)
             if name_and_tag and len(name_and_tag.groups()) == 2:
@@ -179,7 +190,7 @@ def _find_chart_images(self) -> List[Tuple[str, str]]:
                     image_tag,
                     self.name,
                 )
-                images.append((image_name, image_tag))
+                images.add((image_name, image_tag))
             else:
                 logger.warning(
                     "Could not parse image name and tag in line %s in Helm chart %s",
@@ -198,19 +209,20 @@ def _find_image_lines(self, chart: HelmChartInput, image_lines: Set[str]) -> Non
         """
         logger.debug("Finding image lines in Helm chart %s", chart.artifact_name)
         # Find the image lines in the current chart
-        for template in chart.get_templates():
-            for line in template.data:
-                if "image:" in line:
-                    logger.debug(
-                        "Found image line %s in Helm chart %s",
-                        line,
-                        chart.artifact_name,
-                    )
-                    image_lines.add(line.replace("image:", "").strip())
 
-        # Recursively search the dependency charts for image lines
-        for dep in chart.get_dependencies():
-            self._find_image_lines(dep, image_lines)
+        template_lines = []
+
+        if chart.helm_template is not None:
+            template_lines = chart.helm_template.split("\n")
+
+        for line in template_lines:
+            if "image:" in line:
+                logger.debug(
+                    "Found image line %s in Helm chart %s",
+                    line,
+                    chart.artifact_name,
+                )
+                image_lines.add(line.replace("image:", "").strip())
 
     def _generate_artifact_profile(self) -> AzureArcKubernetesArtifactProfile:
         """

src/aosm/azext_aosm/cli_handlers/onboarding_cnf_handler.py

@@ -115,11 +115,9 @@ def _get_processor_list(self) -> [HelmChartProcessor]:
 
     def _validate_helm_template(self):
         """Validate the helm packages."""
-        helm_chart_processors = self._get_processor_list()
-
         validation_errors = {}
 
-        for helm_processor in helm_chart_processors:
+        for helm_processor in self.processors:
             validation_output = helm_processor.input_artifact.validate_template()
 
             if validation_output:

src/aosm/azext_aosm/common/artifact.py

@@ -163,12 +163,26 @@ def upload(self, config: BaseCommonParametersConfig, command_context: CommandCon
             try:
                 oras_client.push(files=[self.file_path], target=target)
                 break
-            except ValueError:
+            except ValueError as error:
                 if retries < 20:
-                    logger.info("Retrying pushing local artifact to ACR. Retries so far: %s", retries)
+                    logger.info(
+                        "Retrying pushing local artifact to ACR. Retries so far: %s",
+                        retries,
+                    )
                     retries += 1
                     sleep(3)
                     continue
+
+                logger.error(
+                    "Failed to upload %s to %s. Check if this image exists in the"
+                    " source registry %s.",
+                    self.file_path,
+                    target,
+                    target_acr,
+                )
+                logger.debug(error, exc_info=True)
+                raise error
+
         logger.info("LocalFileACRArtifact uploaded %s to %s", self.file_path, target)
 
 
@@ -543,10 +557,9 @@ def upload(self, config: VNFCommonParametersConfig, command_context: CommandCont
                 blob_type=BlobType.PAGEBLOB,
                 progress_hook=self._vhd_upload_progress_callback,
             )
+
         logger.info(
-            "Successfully uploaded %s to %s",
-            artifact_config.file_path,
-            self.artifact_client.account_name,
+            "Successfully uploaded %s to %s", self.file_path, blob_client.container_name
         )
 
     def _vhd_upload_progress_callback(

src/aosm/azext_aosm/inputs/helm_chart_input.py

@@ -104,7 +104,7 @@ def __init__(
     @staticmethod
     def from_chart_path(
         chart_path: Path,
-        default_config: Optional[Dict[str, Any]],
+        default_config: Optional[Dict[str, Any]] = None,
         default_config_path: Optional[str] = None,
     ) -> "HelmChartInput":
         """
@@ -173,7 +173,7 @@ def validate_template(self) -> None:
         try:
             result = subprocess.run(cmd, capture_output=True, check=True)
             helm_template_output = result.stdout
-            self.helm_template = helm_template_output
+            self.helm_template = helm_template_output.decode()
 
             logger.debug(
                 "Helm template output for Helm chart %s:\n%s",

src/aosm/azext_aosm/tests/latest/mock_cnf/input-nf-agent-cnf-template.jsonc

@@ -0,0 +1,19 @@
+{
+  "location": "uksouth",
+  "publisher_name": "sunnyclipub",
+  "publisher_resource_group_name": "sunny-uksouth",
+  "nf_name": "nf-agent-cnf",
+  "version": "0.1.0",
+  "acr_artifact_store_name": "sunny-nfagent-acr-2",
+  "images": {
+    "source_registry": "--this was copied here and renamed from https://ms.portal.azure.com/#@microsoft.onmicrosoft.com/resource/subscriptions/4a0479c0-b795-4d0f-96fd-c7edd2a2928f/resourceGroups/pez-nfagent-pipelines/providers/Microsoft.ContainerRegistry/registries/peznfagenttemp/overview new one was /subscriptions/c7bd9d96-70dd-4f61-af56-6e0abd8d80b5/resourceGroups/sunny-nfagent-acr-HostedResources-4CDE264A/providers/Microsoft.ContainerRegistry/registries/SunnyclipubSunnyNfagentAcre00abc1832"
+  },
+  "helm_packages": [
+    {
+      "name": "nf-agent-cnf",
+      "path_to_chart": "{{tests_directory}}/latest/mock_cnf/helm-charts/nf-agent-cnf",
+      "default_values": "",
+      "depends_on": []
+    }
+  ]
+}

src/aosm/azext_aosm/tests/latest/mock_cnf/nf-agent-cnf-helm_template_output.yaml

@@ -0,0 +1,145 @@
+---
+# Source: nf-agent-cnf/templates/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: nf-agent-cnf
+  labels:
+    helm.sh/chart: nf-agent-cnf-0.1.0
+    app.kubernetes.io/name: nf-agent-cnf
+    app.kubernetes.io/instance: nf-agent-cnf
+    app.kubernetes.io/version: "1.16.0"
+    app.kubernetes.io/managed-by: Helm
+---
+# Source: nf-agent-cnf/templates/nf-agent-config-map.yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: nfagent-config
+data:
+  config.yaml: |
+    # Example NF Agent config file.
+    # Config is read from /etc/nf-agent/config.yaml.
+    log_level: debug
+    service_bus:
+      # Using a namespace and Managed Identity (specified by client ID) for auth.
+      namespace: sb-uowvjfivpyuow
+      # Helm uses sprig for templating, so we can use sprig functions to find just the UID from the full Managed Identity ID path.
+      identity: 041db2eb-36e0-42cd-ac13-03ae8e997cd1
+      # Alternatively can use a connstring instead of namespace + managed identity:
+      # connstring: "Endpoint=sb://contoso.servicebus.windows.net/;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=TopSecretSASTokenGoesHere="
+      subscriptions:
+        - topic: simpl
+          subscription: simpl-subscription
+    # Handler-specific config
+    handler_config:
+      simpl:
+        # The endpoint is constructed from the namespace and service name of the receiving thing. 
+        # We couldn't get AOSM to install the service to listen on anything but port 80
+        # Doh - that was because we changed values.yaml in the chart but didn't change values.mappings.yaml in the NFDV
+        # Changing values.mappings.yaml should make this work on port 5222 as expected.
+        endpoint: http://nfconfigchart.nfconfigchart.svc.cluster.local:80 # DevSkim: ignore DS162092
+---
+# Source: nf-agent-cnf/templates/service.yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: nf-agent-cnf
+  labels:
+    helm.sh/chart: nf-agent-cnf-0.1.0
+    app.kubernetes.io/name: nf-agent-cnf
+    app.kubernetes.io/instance: nf-agent-cnf
+    app.kubernetes.io/version: "1.16.0"
+    app.kubernetes.io/managed-by: Helm
+spec:
+  type: ClusterIP
+  ports:
+    - port: 8123
+      targetPort: http
+      protocol: TCP
+      name: http
+  selector:
+    app.kubernetes.io/name: nf-agent-cnf
+    app.kubernetes.io/instance: nf-agent-cnf
+---
+# Source: nf-agent-cnf/templates/deployment.yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nf-agent-cnf
+  labels:
+    helm.sh/chart: nf-agent-cnf-0.1.0
+    app.kubernetes.io/name: nf-agent-cnf
+    app.kubernetes.io/instance: nf-agent-cnf
+    app.kubernetes.io/version: "1.16.0"
+    app.kubernetes.io/managed-by: Helm
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app.kubernetes.io/name: nf-agent-cnf
+      app.kubernetes.io/instance: nf-agent-cnf
+  template:
+    metadata:
+      labels:
+        app.kubernetes.io/name: nf-agent-cnf
+        app.kubernetes.io/instance: nf-agent-cnf
+        #aadpodidbinding:  - not using podidentity any more
+    spec:
+      # Copied imagePullSecrets from how afosas-aosm repo does it
+      imagePullSecrets: []
+      serviceAccountName: nf-agent-cnf
+      securityContext:
+        {}
+      containers:
+        - name: nf-agent-cnf
+          securityContext:
+            {}
+          # Edited the image to point to the nf-agent image in the Artifact Store ACR
+          image: "sunnyclipubsunnynfagentacr2dd56aed266.azurecr.io/pez-nfagent:879624"
+          imagePullPolicy: IfNotPresent
+          ports:
+            - name: http
+              containerPort: 8123
+              protocol: TCP
+          # Commented out otherwise kubernetes keeps restarting the pod thinking the probes have failed
+          # livenessProbe:
+          #   httpGet:
+          #     path: /
+          #     port: http
+          # readinessProbe:
+          #   httpGet:
+          #     path: /
+          #     port: http
+          resources:
+            {}
+          # Gets the NF Agent config from the configMap - see nf-agent-config-map.yaml
+          volumeMounts:
+            - name: nfagent-config-volume
+              mountPath: /etc/nf-agent/config.yaml
+              subPath: config.yaml
+      volumes:
+        - name: nfagent-config-volume
+          configMap:
+            name: nfagent-config
+---
+# Source: nf-agent-cnf/templates/tests/test-connection.yaml
+apiVersion: v1
+kind: Pod
+metadata:
+  name: "nf-agent-cnf-test-connection"
+  labels:
+    helm.sh/chart: nf-agent-cnf-0.1.0
+    app.kubernetes.io/name: nf-agent-cnf
+    app.kubernetes.io/instance: nf-agent-cnf
+    app.kubernetes.io/version: "1.16.0"
+    app.kubernetes.io/managed-by: Helm
+  annotations:
+    "helm.sh/hook": test
+spec:
+  containers:
+    - name: wget
+      image: busybox
+      command: ['wget']
+      args: ['nf-agent-cnf:8123']
+  restartPolicy: Never