feat: adds windows server 2022 #756

nick5616 · 2021-09-27T21:32:03Z

What this PR does / why we need it:
Adds Windows Server 2022 to Makefile, BASEIMAGE, and BASEIMAGE_CORE
Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #

Special notes for your reviewer:
ltsc2022 is the MCR tag for Windows Server 2022
https://hub.docker.com/_/microsoft-windows-nanoserver
There is a PR for the same thing here: kubernetes-sigs/azuredisk-csi-driver#1030

k8s-ci-robot · 2021-09-27T21:32:10Z

Welcome @nick5616!

It looks like this is your first PR to kubernetes-sigs/secrets-store-csi-driver 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/secrets-store-csi-driver has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2021-09-27T21:32:11Z

Hi @nick5616. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

nilekhc · 2021-09-27T21:46:40Z

@nick5616 Thanks for the PR!

/ok-to-test

aramase · 2021-09-27T23:22:58Z

docker/BASEIMAGE

@@ -4,3 +4,5 @@ windows/amd64/1809=mcr.microsoft.com/windows/nanoserver:1809
 windows/amd64/1903=mcr.microsoft.com/windows/nanoserver:1903
 windows/amd64/1909=mcr.microsoft.com/windows/nanoserver:1909
 windows/amd64/2004=mcr.microsoft.com/windows/nanoserver:2004
+windows/amd64/2004=mcr.microsoft.com/windows/nanoserver:ltsc2022
+


nit: remove extra new line

aramase

Thank you for the PR. Just one small nit!

nilekhc · 2021-09-28T02:06:05Z

@aramase @tam7t We are not testing/validating these kind of changes anywhere. Should we add make container-all to our presubmit build job? Perhaps may be a separate presubmit job to validate docker build vs existing build job which validates code build. This way we can be sure of whatever arc/platform we are specifying or adding is always tested. WDYT?

aramase · 2021-09-28T16:04:10Z

@aramase @tam7t We are not testing/validating these kind of changes anywhere. Should we add make container-all to our presubmit build job? Perhaps may be a separate presubmit job to validate docker build vs existing build job which validates code build. This way we can be sure of whatever arc/platform we are specifying or adding is always tested. WDYT?

@nilekhc container-all is used for windows test. We build the multi arch/os image and run tests against that for windows.

docker manifest inspect k8sprow.azurecr.io/driver:v1.0.0-e2e-d4245ff6
{
   "schemaVersion": 2,
   "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
   "manifests": [
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 952,
         "digest": "sha256:0797874d8647fd1ac1f4d7039194cc877db5f37f27a4bdec9f204cadf220333f",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 952,
         "digest": "sha256:e89f2d904e3acdcb69e1f140f8a585cb6fd1fc5daa067e5814c154be43ba03ab",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:51e95d8f7ec36e52b9d242baec871b605fef9c099f92433805d6b32ddf33f69b",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.17763.2183"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:999f6dac5c4dd86b0423db3f9cc1e0017f357a8545b6a793d3699b1244457d5f",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.18362.1256"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:20383a1cd645857c8a1f624ecfeb66dbc3adea71c0600019b197254f7e5f023b",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.18363.1556"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:66fa6fd6dc996fe674d94f70e036ea021e7e4f1c63e080d8b0116892a346f8c9",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.19041.1237"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:8940fd99597ba20e0c08aa97f24b5ecbe0b3fea23d455f383f5a6129d7dbf9a1",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.20348.230"
         }
      }
   ]
}

nilekhc · 2021-09-28T17:22:10Z

@aramase @tam7t We are not testing/validating these kind of changes anywhere. Should we add make container-all to our presubmit build job? Perhaps may be a separate presubmit job to validate docker build vs existing build job which validates code build. This way we can be sure of whatever arc/platform we are specifying or adding is always tested. WDYT?

@nilekhc container-all is used for windows test. We build the multi arch/os image and run tests against that for windows.

docker manifest inspect k8sprow.azurecr.io/driver:v1.0.0-e2e-d4245ff6
{
   "schemaVersion": 2,
   "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
   "manifests": [
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 952,
         "digest": "sha256:0797874d8647fd1ac1f4d7039194cc877db5f37f27a4bdec9f204cadf220333f",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 952,
         "digest": "sha256:e89f2d904e3acdcb69e1f140f8a585cb6fd1fc5daa067e5814c154be43ba03ab",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:51e95d8f7ec36e52b9d242baec871b605fef9c099f92433805d6b32ddf33f69b",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.17763.2183"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:999f6dac5c4dd86b0423db3f9cc1e0017f357a8545b6a793d3699b1244457d5f",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.18362.1256"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:20383a1cd645857c8a1f624ecfeb66dbc3adea71c0600019b197254f7e5f023b",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.18363.1556"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:66fa6fd6dc996fe674d94f70e036ea021e7e4f1c63e080d8b0116892a346f8c9",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.19041.1237"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 951,
         "digest": "sha256:8940fd99597ba20e0c08aa97f24b5ecbe0b3fea23d455f383f5a6129d7dbf9a1",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.20348.230"
         }
      }
   ]
}

Got it. Tests are covered as part of pull-secrets-store-csi-driver-e2e-windows prow infra job. kubetest will build all the windows specific images and test them subsequently.

nilekhc · 2021-09-28T17:23:10Z

/lgtm

/hold
for other reviews..

nilekhc · 2021-09-28T17:37:21Z

docker/BASEIMAGE

@@ -4,3 +4,4 @@ windows/amd64/1809=mcr.microsoft.com/windows/nanoserver:1809
 windows/amd64/1903=mcr.microsoft.com/windows/nanoserver:1903
 windows/amd64/1909=mcr.microsoft.com/windows/nanoserver:1909
 windows/amd64/2004=mcr.microsoft.com/windows/nanoserver:2004
+windows/amd64/2004=mcr.microsoft.com/windows/nanoserver:ltsc2022


nit: please add newline.

Earlier there were two newlines and only one was expected to remove. EOF is expected to have a newline.

nilekhc · 2021-09-28T17:37:29Z

docker/BASEIMAGE_CORE

@@ -2,3 +2,4 @@ windows/amd64/1809=gcr.io/k8s-staging-e2e-test-images/windows-servercore-cache:1
 windows/amd64/1903=gcr.io/k8s-staging-e2e-test-images/windows-servercore-cache:1.0-linux-amd64-1903
 windows/amd64/1909=gcr.io/k8s-staging-e2e-test-images/windows-servercore-cache:1.0-linux-amd64-1909
 windows/amd64/2004=gcr.io/k8s-staging-e2e-test-images/windows-servercore-cache:1.0-linux-amd64-2004
+windows/amd64/ltsc2022=gcr.io/k8s-staging-e2e-test-images/windows-servercore-cache:1.0-linux-amd64-ltsc2022


nit: please add newline.

Removed extra lines

* ci: debug Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * ci: tests inplace upgrade Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * ci: debug Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * test: fixes inplace upgrade test Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * test: removes debug Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> added single new line

…tore-csi-driver into enable-ws2022

aramase · 2021-09-28T18:13:01Z

/label tide/merge-method-squash

@nick5616 this is the label I was referring to for squash ^^

aramase

/lgtm

aramase · 2021-09-28T18:19:42Z

I think we should also cherry pick this to release-1.0 and include in the v1.0.0 release. Thoughts?

nilekhc · 2021-09-28T18:22:00Z

/hold cancel

nilekhc · 2021-09-28T18:24:30Z

/unhold

tam7t · 2021-09-28T18:59:52Z

/approve

k8s-ci-robot · 2021-09-28T19:00:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nick5616, tam7t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [tam7t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

aramase · 2021-09-28T19:02:43Z

@nick5616 Could you also cherry pick this PR to the release-1.0 branch?

Run the following command and that'll create the PR automatically

export GITHUB_USER=<github user name>
hack/cherry_pick_pull.sh upstream/release-1.0 756

* feat: adds windows server 2022 * Removed extra lines * feat: adds windows server 2022 Removed extra lines * fix: inplace upgrade test (#752) * ci: debug Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * ci: tests inplace upgrade Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * ci: debug Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * test: fixes inplace upgrade test Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * test: removes debug Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> added single new line * Revert #752 changes Co-authored-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com>

feat: adds windows server 2022

52ffc26

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 27, 2021

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 27, 2021

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 27, 2021

k8s-ci-robot requested review from aramase and ritazh September 27, 2021 21:32

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 27, 2021

This was referenced Sep 27, 2021

feat: adds windows server 2022 Azure/secrets-store-csi-driver-provider-azure#660

Merged

feat: Adds windows server 2022 kubernetes-sigs/cloud-provider-azure#821

Merged

aramase reviewed Sep 27, 2021

View reviewed changes

Nicolas Belovoskey added 2 commits September 28, 2021 10:16

Removed extra lines

3478498

Merge branch 'kubernetes-sigs:main' into enable-ws2022

32c5a7c

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 28, 2021

k8s-ci-robot assigned nilekhc Sep 28, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 28, 2021

nilekhc reviewed Sep 28, 2021

View reviewed changes

added single new line

1cf9ef0

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 28, 2021

Nicolas Belovoskey and others added 3 commits September 28, 2021 10:51

feat: adds windows server 2022

b8209a9

Removed extra lines

Merge branch 'enable-ws2022' of https://github.com/nick5616/secrets-s…

1bfd663

…tore-csi-driver into enable-ws2022

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Sep 28, 2021

aramase reviewed Sep 28, 2021

View reviewed changes

k8s-ci-robot assigned aramase Sep 28, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 28, 2021

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 28, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 28, 2021

k8s-ci-robot merged commit ef12a51 into kubernetes-sigs:main Sep 28, 2021

nick5616 mentioned this pull request Sep 28, 2021

Automated cherry pick of #756: feat: adds windows server 2022 #757

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds windows server 2022 #756

feat: adds windows server 2022 #756

nick5616 commented Sep 27, 2021

k8s-ci-robot commented Sep 27, 2021

k8s-ci-robot commented Sep 27, 2021

nilekhc commented Sep 27, 2021

aramase Sep 27, 2021

aramase left a comment

nilekhc commented Sep 28, 2021

aramase commented Sep 28, 2021

nilekhc commented Sep 28, 2021

nilekhc commented Sep 28, 2021

nilekhc Sep 28, 2021

nilekhc Sep 28, 2021

aramase commented Sep 28, 2021

aramase left a comment

aramase commented Sep 28, 2021

nilekhc commented Sep 28, 2021 •

edited

nilekhc commented Sep 28, 2021

tam7t commented Sep 28, 2021

k8s-ci-robot commented Sep 28, 2021

aramase commented Sep 28, 2021

feat: adds windows server 2022 #756

feat: adds windows server 2022 #756

Conversation

nick5616 commented Sep 27, 2021

k8s-ci-robot commented Sep 27, 2021

k8s-ci-robot commented Sep 27, 2021

nilekhc commented Sep 27, 2021

aramase Sep 27, 2021

Choose a reason for hiding this comment

aramase left a comment

Choose a reason for hiding this comment

nilekhc commented Sep 28, 2021

aramase commented Sep 28, 2021

nilekhc commented Sep 28, 2021

nilekhc commented Sep 28, 2021

nilekhc Sep 28, 2021

Choose a reason for hiding this comment

nilekhc Sep 28, 2021

Choose a reason for hiding this comment

aramase commented Sep 28, 2021

aramase left a comment

Choose a reason for hiding this comment

aramase commented Sep 28, 2021

nilekhc commented Sep 28, 2021 • edited

nilekhc commented Sep 28, 2021

tam7t commented Sep 28, 2021

k8s-ci-robot commented Sep 28, 2021

aramase commented Sep 28, 2021

nilekhc commented Sep 28, 2021 •

edited