Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Achurchard/fix helm chart upload #144

Merged
merged 3 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/aosm/azext_aosm/build_processors/nfd_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def get_artifact_details(
# Path is relative to NSD_OUTPUT_FOLDER_FILENAME as this artifact is stored in the NSD output folder
artifact_details = LocalFileACRArtifact(
artifact_name=self.input_artifact.artifact_name,
artifact_type=ArtifactType.OCI_ARTIFACT.value,
artifact_type=ArtifactType.ARM_TEMPLATE.value,
artifact_version=self.input_artifact.artifact_version,
file_path=self.input_artifact.arm_template_output_path.relative_to(
Path(NSD_OUTPUT_FOLDER_FILENAME)
Expand Down
119 changes: 98 additions & 21 deletions src/aosm/azext_aosm/common/artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
BlobClient,
BlobType,
)
from azext_aosm.vendored_sdks.models import ArtifactType
from azext_aosm.vendored_sdks import HybridNetworkManagementClient
from azext_aosm.common.command_context import CommandContext
from azext_aosm.common.utils import convert_bicep_to_arm
Expand Down Expand Up @@ -186,32 +187,108 @@ def upload(
target_acr = self._get_acr(oras_client)
target = f"{target_acr}/{self.artifact_name}:{self.artifact_version}"
logger.debug("Uploading %s to %s", self.file_path, target)
retries = 0
while True:
try:
oras_client.push(files=[self.file_path], target=target)
break
except ValueError as error:
if retries < 20:
logger.info(
"Retrying pushing local artifact to ACR. Retries so far: %s",
retries,

if self.artifact_type == ArtifactType.ARM_TEMPLATE.value:
retries = 0
while True:
try:
oras_client.push(files=[self.file_path], target=target)
break
except ValueError as error:
if retries < 20:
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,
)
retries += 1
sleep(3)
continue
logger.debug(error, exc_info=True)
raise error

logger.error(
"Failed to upload %s to %s. Check if this image exists in the"
" source registry %s.",
logger.info("LocalFileACRArtifact uploaded %s to %s using oras push", self.file_path, target)

elif self.artifact_type == ArtifactType.OCI_ARTIFACT.value:

target_acr_name = target_acr.replace(".azurecr.io", "")
target_acr_with_protocol = f"oci://{target_acr}"
username = manifest_credentials["username"]
password = manifest_credentials["acr_token"]

# TODO: Maybe port over the "if not use_manifest_permissions" feature which means you don't need to
# install docker. Although not having to install docker feels like a marginal enhancement, as most
# people playing with containers will have docker, or won't mind installing it. Note that
# use_manifest_permissions is now in command_context.cli_options['no_subscription_permissions']

self._check_tool_installed("docker")
self._check_tool_installed("helm")

# TODO: don't just dump this in /tmp
if self.file_path.is_dir():
helm_package_cmd = [
str(shutil.which("helm")),
"package",
self.file_path,
target,
"--destination",
"/tmp",
]
self._call_subprocess_raise_output(helm_package_cmd)

# This seems to prevent occasional helm login failures
acr_login_cmd = [
str(shutil.which("az")),
"acr",
"login",
"--name",
target_acr_name,
"--username",
username,
"--password",
password,
]
self._call_subprocess_raise_output(acr_login_cmd)

try:
helm_login_cmd = [
str(shutil.which("helm")),
"registry",
"login",
target_acr,
)
logger.debug(error, exc_info=True)
raise error
"--username",
username,
"--password",
password,
]
self._call_subprocess_raise_output(helm_login_cmd)

push_command = [
str(shutil.which("helm")),
"push",
f"/tmp/{self.artifact_name}-{self.artifact_version}.tgz", # TODO: fix up helm package to use non-tmp path
target_acr_with_protocol,
]
self._call_subprocess_raise_output(push_command)
finally:
helm_logout_cmd = [
str(shutil.which("helm")),
"registry",
"logout",
target_acr,
]
self._call_subprocess_raise_output(helm_logout_cmd)

logger.info("LocalFileACRArtifact uploaded %s to %s using helm push", self.file_path, target)

logger.info("LocalFileACRArtifact uploaded %s to %s", self.file_path, target)
else: # TODO: Make this one of the allowed Azure CLI exceptions
raise ValueError(f"Unexpected artifact type. Got {self.artifact_type}. Expected {ArtifactType.ARM_TEMPLATE.value} or {ArtifactType.OCI_ARTIFACT.value}")


class RemoteACRArtifact(BaseACRArtifact):
Expand Down