Skip to content

Remove internal GCS connection functionality#1038

Merged
dcantah merged 1 commit intomicrosoft:masterfrom
dcantah:remove-internal-guestconn
May 29, 2021
Merged

Remove internal GCS connection functionality#1038
dcantah merged 1 commit intomicrosoft:masterfrom
dcantah:remove-internal-guestconn

Conversation

@dcantah
Copy link
Contributor

@dcantah dcantah commented May 27, 2021

HCS maintains an internal guest connection to the GCS normally if you request it.
However, there are certain features that require us to maintain an external connection
(external in this sense meaning not in HCS) instead like late cloning.

We had swapped to always managing the connection to the GCS ourselves some time ago and
afaik there's been no fallout from it, so I propose let's get rid of the internal branches
altogether. This greatly simplifies the work for going through a different virtstack for
hypervisor isolated containers as well.

Ran go mod vendor in /test to bring in the changes as well.

Signed-off-by: Daniel Canter dcanter@microsoft.com

@dcantah dcantah requested a review from a team as a code owner May 27, 2021 19:03
HCS maintains an internal guest connection to the GCS normally if you request it.
However, there are certain features that require us to maintain an external connection
(external in this sense meaning not in HCS) instead like late cloning.

We had swapped to always managing the connection to the GCS ourselves some time ago and
afaik there's been no fallout from it, so I propose let's get rid of the internal branches
altogether. This greatly simplifies the work for going through a different virtstack for
hypervisor isolated containers as well.

Ran go mod vendor in /test to bring in the changes as well.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@katiewasnothere
Copy link

did you run the cri-containerd tests for this change?

@dcantah
Copy link
Contributor Author

dcantah commented May 28, 2021

@katiewasnothere Yea, there's basically no change as it's the default already, I removed the one test (that I could find) that resorted to internal

Copy link

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

lgtm

@dcantah dcantah merged commit 5558027 into microsoft:master May 29, 2021
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