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

Add Windows 20H2 images #523

Merged
merged 1 commit into from Apr 12, 2021
Merged

Conversation

vboulineau
Copy link
Contributor

PR Summary

Add images based on Windows 20H2 images.

PR Checklist

@ghost
Copy link

ghost commented Jan 5, 2021

CLA assistant check
All CLA requirements met.

Copy link
Collaborator

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

Hey @vboulineau,
I looked over the PR and found a few things to note, so here they are.
Thanks for the contribution!

@@ -3,4 +3,4 @@

# Dummy docker image to trigger dependabot PRs

FROM mcr.microsoft.com/windows/nanoserver:2004
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like nano 2k4 should be kept for LTS, and only introduce this new version in stable and preview channels, since it is new. Even if the LTS supports it, it seems like its a bit late in the LTS lifespan to introduce a new OS version now.

Copy link
Member

Choose a reason for hiding this comment

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

This change is fine, as long as 2004 image is not deleted

@@ -14,6 +14,9 @@
},
{
"Tag": "2004"
},
{
"Tag": "20H2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, this is servicing. We should not have it here 100%, but maybe for LTS if @TravisEz13 agrees.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Works for me, removed for servicing. Pending review for LTS.

Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

Pending test run

@@ -3,4 +3,4 @@

# Dummy docker image to trigger dependabot PRs

FROM mcr.microsoft.com/windows/nanoserver:2004
Copy link
Member

Choose a reason for hiding this comment

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

This change is fine, as long as 2004 image is not deleted

@TravisEz13
Copy link
Member

/azp list

@azure-pipelines
Copy link

CI/CD Pipelines for this repository:

@TravisEz13
Copy link
Member

/azp run docker-ACR-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@TravisEz13
Copy link
Member

@PoshChan Please remind me in 1 hour

@PoshChan
Copy link
Collaborator

@TravisEz13, this is the reminder you requested 1 hour ago

@luthermonson
Copy link

@TravisEz13 we still have a chance of getting this today?

@RDIL
Copy link
Collaborator

RDIL commented Jan 16, 2021

The build is currently failing so no.

@vboulineau
Copy link
Contributor Author

Hello @RDIL @TravisEz13, the failure seems to be due to az acr build --platform windows running at most 10.0.19041 (so 2004).
I am not sure there's a straightforward workaround for this, let me know if there's anything I need to change on my side.

@ghost ghost removed the Waiting on Author label Jan 18, 2021
@RDIL
Copy link
Collaborator

RDIL commented Jan 18, 2021

That's not good! I guess we have to wait for Azure then 😢

@jeanfrancoislarente
Copy link

Ping on this one - would be great to get this soon

@RDIL
Copy link
Collaborator

RDIL commented Jan 20, 2021

Ping on this one - would be great to get this soon

Hey,
I know this is a highly requested feature. We will try to get this out whenever we can.
Sorry for the wait!

Copy link
Collaborator

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

Requested changes are fixed now

@jeanfrancoislarente
Copy link

Ping on this one - would be great to get this soon

Hey,
I know this is a highly requested feature. We will try to get this out whenever we can.
Sorry for the wait!

Not complaining, just asking :)

It's important for many who follow the SAC releases closely to have "all the things" follow along as well!

@TravisEz13
Copy link
Member

We cannot release until az acr can build 20h2. I'll try again today.

@TravisEz13

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@TravisEz13

This comment has been minimized.

@aravindhp
Copy link

Gentle ping on this PR

@TravisEz13
Copy link
Member

@aravindhp Still waiting on ACR. No current ETA.

@TravisEz13
Copy link
Member

The following issue has been filed with ACR to track this: Azure/acr#532

@TravisEz13

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

Copy link
Member

@anamnavi anamnavi left a comment

Choose a reason for hiding this comment

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

Hi @vboulineau, I added IsBroken tags so we can get this in even though we can't publish these to ACR til we have resolution from that side. If you could pull in changes from upstream master (as your branch is 8 commits behind PowerShell:master) we should be good to merge I believe.

Signed-off-by: Vincent Boulineau <vincent.boulineau@datadoghq.com>
@vboulineau
Copy link
Contributor Author

Hi @vboulineau, I added IsBroken tags so we can get this in even though we can't publish these to ACR til we have resolution from that side. If you could pull in changes from upstream master (as your branch is 8 commits behind PowerShell:master) we should be good to merge I believe.

Done, I guess it will require to manually remove the IsBroken metadata when support is available and finally get a released image.

@TravisEz13
Copy link
Member

/azp run docker-ACR-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@TravisEz13
Copy link
Member

@PoshChan Please remind me in 1 hour

Copy link
Member

@anamnavi anamnavi left a comment

Choose a reason for hiding this comment

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

built the images again to test and check and all looks good to me. Tests look good too.

@PoshChan
Copy link
Collaborator

@TravisEz13, this is the reminder you requested 1 hour ago

@anamnavi anamnavi merged commit 890b315 into PowerShell:master Apr 12, 2021
@anamnavi
Copy link
Member

Hi @vboulineau, I added IsBroken tags so we can get this in even though we can't publish these to ACR til we have resolution from that side. If you could pull in changes from upstream master (as your branch is 8 commits behind PowerShell:master) we should be good to merge I believe.

Done, I guess it will require to manually remove the IsBroken metadata when support is available and finally get a released image.

Yes, I'll come back and make those changes to the metadata when support is finally available. Thank you again for the contribution and patience as we figured this out :)

@donny-dont
Copy link

@anamnavi can we have an issue to follow for when 20H2 support is finally available?

@anamnavi
Copy link
Member

sure @donny-dont. Here's the issue: #552

@TravisEz13
Copy link
Member

TravisEz13 commented Apr 12, 2021

I also added a known issue to the wiki.
https://github.com/PowerShell/PowerShell-Docker/wiki/Known-Issues#windows-20h2-images-and-newer

GitHub
Repository for building PowerShell Docker images. Contribute to PowerShell/PowerShell-Docker development by creating an account on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants