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.

Sign up for GitHub

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

Jump to bottom

Remove k8s dependency to circumvent recursive import #783

Merged
merged 1 commit into from
Mar 11, 2020
Merged

Remove k8s dependency to circumvent recursive import #783

merged 1 commit into from
Mar 11, 2020

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Mar 7, 2020 •
edited

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

@dcantah dcantah requested a review from a team March 7, 2020 02:56
@dcantah dcantah mentioned this pull request Mar 9, 2020
Closed
@ambarve
Copy link
Contributor

ambarve commented Mar 10, 2020

@dcantah Could you please add a little more details to the PR about the issue and the general description of the fix? It will be easier to review this with some more context.

@katiewasnothere
Copy link
Contributor

@ambarve the issue that this PR is mentioned in above has some of the context :) @dcantah could you still add some information about what this PR addresses from that issue to make it more clear :)

@ambarve
Copy link
Contributor

ambarve commented Mar 10, 2020

@katiewasnothere My bad. I didn't realize that the issue was linked here. Thanks.

@dcantah
Copy link
Contributor Author

dcantah commented Mar 10, 2020

Will update the commit msg to link to the relevant issue/reason for the pr :)

@kevpar
Copy link
Member

kevpar commented Mar 10, 2020

I'm confused why we see so many changes in vendor/modules.txt. Did you just replace the import paths, fix the ReopenContainerLogs call, build, and run go mod vendor?

@dcantah
Copy link
Contributor Author

dcantah commented Mar 10, 2020

@kevpar correct, except the last step was running go mod tidy.

@jstarks
Copy link
Member

jstarks commented Mar 10, 2020

Why is gogo/protobuf updated to an unreleased version as part of this change?

@dcantah
Remove k8s dependency to circumvent recursive import
ddc3095 
* kubernetes/kubernetes#87420
* This is to fix the above issue regarding the recursive dependency of kubernetes importing the shim and the shim importing
kubernetes when all we used it for was the cri package (and the fact it's only used for tests).
* Use k8s.io/cri-api for tests instead of k8s.io/kubernetes/pkg/kubelet/apis/cri
* Bump gogo/protobuf version to 1.3.1 and grpc to 1.23.1

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

kevpar commented Mar 11, 2020

Why is gogo/protobuf updated to an unreleased version as part of this change?

@jstarks looks like k8s.io/cri-api@0.17.3 lists that version in its go.mod, so the main module here was upgraded to pick the latest version that anything in its dependency tree uses.

@dcantah
Copy link
Contributor Author

dcantah commented Mar 11, 2020 •
edited

Why is gogo/protobuf updated to an unreleased version as part of this change?

@jstarks looks like k8s.io/cri-api@0.17.3 lists that version in its go.mod, so the main module here was upgraded to pick the latest version that anything in its dependency tree uses.

@kevpar Yep, we talked about this earlier today but I forgot to answer it here as well so thank you!

@kevpar
Copy link
Member

kevpar commented Mar 11, 2020

This resolves #760.

@kevpar
Copy link
Member

kevpar commented Mar 11, 2020

Have you validated that the tests still pass with this change?

@dcantah
Copy link
Contributor Author

dcantah commented Mar 11, 2020

@kevpar yep!

kevpar
kevpar approved these changes Mar 11, 2020
View reviewed changes
Copy link
Member

@kevpar kevpar 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 af6d13f into microsoft:master Mar 11, 2020
@dims dims mentioned this pull request Mar 26, 2020
Closed
justaugustus added a commit to justaugustus/release that referenced this pull request Mar 1, 2021
@justaugustus
go.mod: Update github.com/containers/image/v5 to v5.10.4
04c839a 
containers/storage@v1.23.7 is imported by containers/image@v5.9.0, which
imports Microsoft/hcsshim@v0.8.9, which imports kubernetes/kubernetes,
which is not recommended for any consumer of Kubernetes and its
subproject libraries.

This was fixed in microsoft/hcsshim#783, so
let's pull in a newer version of containers/image that reflects this
change.

Ultimately, this was _rediscovered_ when we replaced the verify
dependencies script in kubernetes/kubernetes with sigs.k8s.io/zeitgeist,
which imports k/release.

Signed-off-by: Stephen Augustus <foo@auggie.dev>
@justaugustus justaugustus mentioned this pull request Mar 1, 2021
Merged
wespanther pushed a commit to wespanther/release that referenced this pull request Mar 15, 2021
@justaugustus @wespanther
go.mod: Update github.com/containers/image/v5 to v5.10.4
3d7b308 
containers/storage@v1.23.7 is imported by containers/image@v5.9.0, which
imports Microsoft/hcsshim@v0.8.9, which imports kubernetes/kubernetes,
which is not recommended for any consumer of Kubernetes and its
subproject libraries.

This was fixed in microsoft/hcsshim#783, so
let's pull in a newer version of containers/image that reflects this
change.

Ultimately, this was _rediscovered_ when we replaced the verify
dependencies script in kubernetes/kubernetes with sigs.k8s.io/zeitgeist,
which imports k/release.

Signed-off-by: Stephen Augustus <foo@auggie.dev>
zeeke added a commit to zeeke/cnf-features-deploy that referenced this pull request Feb 9, 2024
@zeeke
ztp: Downgrade MCO to an ignition version v3.2.0
c1c2c3a 
ZTP must produce MachineConfig resources with ignition version v3.2.0
Since openshift/machine-config-operator#3814, MCO writes v3.4.0 ignition configuration.
openshift/machine-config-operator@63d7be1 is the commit just before the culprit one.

Also, machine-config-operator@63d7be1ef18b86826b47c61172c7a9dc7c2b6de1
has a transitive dependency to `github.com/Microsoft/hcsshim@v0.8.7` that cause
a `go mod tidy` error:

```
github.com/Microsoft/hcsshim@v0.8.7 requires
k8s.io/kubernetes@v1.13.0 requires
k8s.io/endpointslice@v0.0.0: reading k8s.io/endpointslice/go.mod at revision v0.0.0: unknown revision v0.0.0
```
which is fixed in v0.8.8 by microsoft/hcsshim#783

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
@zeeke zeeke mentioned this pull request Feb 9, 2024
Merged
zeeke added a commit to zeeke/cnf-features-deploy that referenced this pull request Feb 13, 2024
@zeeke
ztp: Downgrade MCO to an ignition version v3.2.0
505fb27 
ZTP must produce MachineConfig resources with ignition version v3.2.0
Since openshift/machine-config-operator#3814, MCO writes v3.4.0 ignition configuration.
openshift/machine-config-operator@63d7be1 is the commit just before the culprit one.

Also, machine-config-operator@63d7be1ef18b86826b47c61172c7a9dc7c2b6de1
has a transitive dependency to `github.com/Microsoft/hcsshim@v0.8.7` that cause
a `go mod tidy` error:

```
github.com/Microsoft/hcsshim@v0.8.7 requires
k8s.io/kubernetes@v1.13.0 requires
k8s.io/endpointslice@v0.0.0: reading k8s.io/endpointslice/go.mod at revision v0.0.0: unknown revision v0.0.0
```
which is fixed in v0.8.8 by microsoft/hcsshim#783

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
zeeke added a commit to zeeke/cnf-features-deploy that referenced this pull request Feb 19, 2024
@zeeke
ztp: Downgrade MCO to an ignition version v3.2.0
516bcc0 
ZTP must produce MachineConfig resources with ignition version v3.2.0
Since openshift/machine-config-operator#3814, MCO writes v3.4.0 ignition configuration.
openshift/machine-config-operator@63d7be1 is the commit just before the culprit one.

Also, machine-config-operator@63d7be1ef18b86826b47c61172c7a9dc7c2b6de1
has a transitive dependency to `github.com/Microsoft/hcsshim@v0.8.7` that cause
a `go mod tidy` error:

```
github.com/Microsoft/hcsshim@v0.8.7 requires
k8s.io/kubernetes@v1.13.0 requires
k8s.io/endpointslice@v0.0.0: reading k8s.io/endpointslice/go.mod at revision v0.0.0: unknown revision v0.0.0
```
which is fixed in v0.8.8 by microsoft/hcsshim#783

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevpar kevpar kevpar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dcantah @ambarve @katiewasnothere @kevpar @jstarks