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

Oryx Issue #1181

Merged
merged 9 commits into from
Dec 3, 2024
Merged

Oryx Issue #1181

merged 9 commits into from
Dec 3, 2024

Conversation

Mathiyarasy
Copy link
Contributor

@Mathiyarasy Mathiyarasy commented Nov 14, 2024

Universal Image Latest Version: Caused issue in a Pipeline
Azure DevOps Pipeline generate-kitchensink-automated Failed

This PR aims to do things:

  1. The pinned dotnet version now is 8.0.202 : Oryx: Unpin .NET version 8.0.101
  2. Publish oryx app with --self-contained true tag . Without this tag oryx fails to launch stating compatible .net runtime version not found

@Mathiyarasy Mathiyarasy marked this pull request as ready for review November 15, 2024 08:15
@Mathiyarasy Mathiyarasy requested a review from a team as a code owner November 15, 2024 08:15
src/oryx/install.sh Outdated Show resolved Hide resolved
src/oryx/install.sh Outdated Show resolved Hide resolved
src/oryx/install.sh Show resolved Hide resolved
@Mathiyarasy
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Microsoft"

eljog
eljog previously approved these changes Nov 26, 2024
src/oryx/install.sh Show resolved Hide resolved
src/oryx/install.sh Show resolved Hide resolved
@eljog
Copy link
Member

eljog commented Nov 27, 2024

@Mathiyarasy please take a look at failing checks/tests

@Mathiyarasy
Copy link
Contributor Author

Mathiyarasy commented Nov 28, 2024

Looked into the failing tests
Oryx do not support Jammy. Unsupported distribution version 'jammy'. is a valid failure scenario. The following 2 failures are valid and expected

  1. test (oryx, mcr.microsoft.com/devcontainers/base:ubuntu)
  2. test (ubuntu:jammy)

In test scenarios oryx install_dotnet_and_oryx fails at test case 'two versions of dotnet runtimes are present'.
Issue is because existing code is not removing dotnet runtime installed with SDK.
SDK: 8.0.202 installs Runtime: 8.0.3
But existing code tried to remove 8.0.2
Modified the code to identify the runtime installed with sdk in oryx . Test case is passing now.

src/oryx/install.sh Outdated Show resolved Hide resolved
eljog
eljog previously approved these changes Dec 2, 2024
@eljog
Copy link
Member

eljog commented Dec 2, 2024

We should update the feature version as well, right? @Mathiyarasy

if [ -n "${RUNTIME_VERSIONS:-}" ]; then
SDK_INSTALLED_RUNTIME=$(echo "$NEW_RUNTIME_VERSIONS" | grep -vxFf <(echo "$RUNTIME_VERSIONS"))
else
SDK_INSTALLED_RUNTIME="$NEW_RUNTIME_VERSIONS"
Copy link
Member

Choose a reason for hiding this comment

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

Are there cases where more than one sdk is installed? I see plural NEW_RUNTIME_VERSIONS
In that case SDK_INSTALLED_RUNTIME may get assigned a multiline string? Wouldn't that case issues with deletions on the lines below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we install only one sdk as a part of oryx and this is installing only one runtime
It is in plural because for images with dotnet even before oryx they sometime might have multiple runtimes from those runtimes we pick only the runtime installed from oryx feature.

Copy link
Member

Choose a reason for hiding this comment

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

In that case should the naming be updated to avoid the confusion?
I will go ahead and checkin this PR, you may followup with a PR if needed

@eljog eljog merged commit f8e7e27 into devcontainers:main Dec 3, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants