Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Bump CNI Plugins to v1.0.1 #863

Closed
wants to merge 6 commits into from
Closed

Bump CNI Plugins to v1.0.1 #863

wants to merge 6 commits into from

Conversation

gaby
Copy link
Contributor

@gaby gaby commented Aug 12, 2021

CNI Plugins have officially graduated to stable with version 1.0.1 released yesterday. The biggest breaking change is the removal of the flannel plugin.

Release notes here: https://github.com/containernetworking/plugins/releases/tag/v1.0.1

CNI plugins have graduated to stable starting with version 1.0.0
@gaby
Copy link
Contributor Author

gaby commented Aug 12, 2021

Doing a search for flannel on this repo shows several mentions. Does this mean that now flannel has to be included as part of ignite or should the change not affect ignite?

@stealthybox
Copy link
Contributor

Thanks @gaby ! Good catch
Ignite doesn't directly depend on the flannel CNI plugin, so we are safe there.

We do have a guide for setting up networking with Flannel -- this needs to be updated to note that the flannel CNI plugin should be installed before this section: https://ignite.readthedocs.io/en/stable/networking/#configuring-the-nodes

The script for that tutorial depends on it here: https://github.com/weaveworks/ignite/blob/b5214a0/tools/ignite-flannel.sh#L20

@darkowlzz darkowlzz added area/dependency Issues or PRs related to dependency changes area/networking Issues related to networking labels Aug 16, 2021
@stealthybox
Copy link
Contributor

I see I personally forgot to bump the go.mod in #805 and #731, but luxas did in #561.
Looking at updating go.mod now to v1.0.1, @darkowlzz noticed there may be a dependency conflict with firecracker-go-sdk.

The most current fc-go-sdk commit is @ cni plugins v0.9.0 (https://github.com/firecracker-microvm/firecracker-go-sdk/blob/90688a4/go.mod#L8) but we are many commits behind on that dep as well.

@stealthybox
Copy link
Contributor

@darkowlzz is going to look at PR'ing firecracker-microvm/firecracker-go-sdk to update containernetworking/plugins. They depend on a package that has moved.

@gaby
Copy link
Contributor Author

gaby commented Aug 17, 2021

The flannel cni plugin now lives here: https://github.com/flannel-io/cni-plugin

Sadly they haven't done a release yet, so no binaries available.

@darkowlzz
Copy link
Contributor

darkowlzz commented Aug 26, 2021

Hi, I've created firecracker-microvm/firecracker-go-sdk#351 to update CNI dependencies in firecracker-go-sdk and also another PR to tc-redirect-tap repo which fc-go-sdk depends on. I was able to locally run the tests in those repos with those changes successfully. I hope they get accepted and we can go ahead with this PR.

We should also update the default CNI config in

"cniVersion": "0.4.0",
to 1.0.0.

@gaby
Copy link
Contributor Author

gaby commented Aug 27, 2021

Seems like changing the default config version breaks CNI.

time="2021-08-27T04:21:26Z" level=error msg="failed to setup network for namespace \"ignite-1ad06c2be2bbbd49\": unsupported CNI result version \"1.0.0\""

@darkowlzz
Copy link
Contributor

Seems like changing the default config version breaks CNI.

@gaby that's expected for now because we are still using a different version of CNI go dependencies, not v1.0.0. It'll be fine once we have updated the go dependencies.

@gaby
Copy link
Contributor Author

gaby commented Aug 27, 2021

It's related to CNI plugins itself, there's a PR for fixing this.

containernetworking/plugins#652

@gaby
Copy link
Contributor Author

gaby commented Sep 7, 2021

CNI plugins 1.0.1 was just released which fixes the config file version!

https://github.com/containernetworking/plugins/releases/tag/v1.0.1

@jhult
Copy link
Contributor

jhult commented Oct 22, 2021

@stealthybox and @darkowlzz, any news on getting this merged?

@gaby
Copy link
Contributor Author

gaby commented Oct 22, 2021

@jhult Thanks for the reminder. I just pushed changes for v1.0.1 which official bumps the SPEC to 1.0.0 as stable.

I think that firecracker-microvm/firecracker-go-sdk#351 needs to get merge before this PR can be merge.

The firecracker PR needs awslabs/tc-redirect-tap#9 to be merge

@gaby gaby changed the title Bump CNI Plugins to v1.0.0 Bump CNI Plugins to v1.0.1 Oct 22, 2021
@gaby
Copy link
Contributor Author

gaby commented Oct 23, 2021

Submitted awslabs/tc-redirect-tap#11 to address the breaking bug in v1.0.0

@jhult
Copy link
Contributor

jhult commented Nov 7, 2021

@gaby
Copy link
Contributor Author

gaby commented Nov 7, 2021

@jhult The CI needs to be re-triggered.

@darkowlzz
Copy link
Contributor

@gaby looking at the CI failure, https://weaveworks-foss.semaphoreci.com/jobs/6eb41f54-1bcd-4e6b-a94a-d08c49eaa109, it could be that the go dependencies need to be updated as well.

@gaby
Copy link
Contributor Author

gaby commented Nov 14, 2021

Ahhh! I'd take a look tomorrow

@gaby
Copy link
Contributor Author

gaby commented Nov 24, 2021

I really have no idea how to update these, Go makes this way too complicated 😆

@gaby
Copy link
Contributor Author

gaby commented Nov 24, 2021

If someone can point me out on how to update Go dependencies, I'd really appreciate it. Everything I try locally/github fails to build.

@darkowlzz
Copy link
Contributor

@gaby The changes in d367a2e looks right. If go modules removes things from go.sum, no need to add it back manually.
Maybe you just need to update the vendored dependencies with make tidy-in-docker. That'll update all the relevant files in the vendor directory.

@gaby
Copy link
Contributor Author

gaby commented Nov 25, 2021

Current output of running make tidy-in-docker

go: finding module for package github.com/containernetworking/cni/pkg/types/current
github.com/weaveworks/ignite/pkg/container imports
	github.com/firecracker-microvm/firecracker-go-sdk imports
	github.com/containernetworking/cni/pkg/types/current: module github.com/containernetworking/cni@latest found (v1.0.1), but does not contain package github.com/containernetworking/cni/pkg/types/current

I remember @darkowlzz doing changes regarding that package in firecracker-microvm/firecracker-go-sdk#351

@gaby
Copy link
Contributor Author

gaby commented Nov 25, 2021

It seems like https://github.com/firecracker-microvm/firecracker-go-sdk may need another PR.

Output:

ubuntu@ubuntu-vb:~/Desktop/ignite$ go get github.com/firecracker-microvm/firecracker-go-sdk
github.com/firecracker-microvm/firecracker-go-sdk imports
	github.com/containernetworking/cni/pkg/types/current: cannot find module providing package github.com/containernetworking/cni/pkg/types/current

@darkowlzz
Copy link
Contributor

@gaby looking at the go.mod file, the version of firecracker-go-sdk is v0.22.0, which is more than a year old.
To update it, you can edit go.mod file's firecracker-go-sdk require line to be:

...
github.com/firecracker-microvm/firecracker-go-sdk 3de2ae41e930c8ff1d726da9f513df11957f4de3
...

or the equivalent revision from the main branch of that repo.
And then run tidy again. That should update everything accordingly.
If that doesn't help, let me know. I can give it a try at resolving this.

@abn abn mentioned this pull request Mar 7, 2022
@gaby gaby closed this Apr 27, 2022
@gaby gaby deleted the update-cni-1.0 branch April 27, 2022 02:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/dependency Issues or PRs related to dependency changes area/networking Issues related to networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants