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

Use containerd as default container runtime socket for standard Kubernetes #508

Merged
merged 7 commits into from
Sep 8, 2022
Merged

Use containerd as default container runtime socket for standard Kubernetes #508

merged 7 commits into from
Sep 8, 2022

Conversation

Rishit-dagli
Copy link
Contributor

@Rishit-dagli Rishit-dagli commented Sep 7, 2022 •
edited
Loading

What this PR does / why we need it:

Akri by default assumes the container runtime socket to be at /var/run/dockershim.sock which should be changed considering the deprecation of Docker as the underlying runtime.

Closes #455

Special notes for your reviewer:

If applicable:

  • this PR has an associated PR with documentation in akri-docs
  • this PR contains unit tests
  • added code adheres to standard Rust formatting (cargo fmt)
  • code builds properly (cargo build)
  • code is free of common mistakes (cargo clippy)
  • all Akri tests succeed (cargo test)
  • inline documentation builds (cargo doc)
  • version has been updated appropriately (./version.sh)
  • all commits pass the DCO bot check by being signed off -- see the failing DCO check for instructions on how to retroactively sign commits

@Rishit-dagli
Update tests
e8a7f96 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli
Update helm templates
aded496 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli
Update helm values
8036803 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli
Update helm lint commands
cad1534 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli
Updates tests
0e5fa18 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli Rishit-dagli force-pushed the Rishit-dagli-containerd branch from bcc3600 to 0e5fa18 Compare September 7, 2022 06:04
@Rishit-dagli
Copy link
Contributor Author

I haven't updated the version yet, I think we would put this as a breaking change, right?

@Rishit-dagli Rishit-dagli mentioned this pull request Sep 7, 2022
Merged
@kate-goldenring
Copy link
Contributor

I haven't updated the version yet, I think we would put this as a breaking change, right?

Good question. I think a patch version bump could be sufficient given that you can still configure the path in the Helm charts to point to docker.

@kate-goldenring
Copy link
Contributor

Let's see how the e2e test go but this looks great @Rishit-dagli! If you are interested in updating the docs too, here is the associated doc: https://github.com/project-akri/akri-docs/blob/main/docs/user-guide/cluster-setup.md

@Rishit-dagli
Copy link
Contributor Author

Rishit-dagli commented Sep 7, 2022 •
edited
Loading

Let's see how the e2e test go

This is from my fork, so we should also wait for the tests here. But here are the test results: https://github.com/Rishit-dagli/akri/actions/runs/3002656848 . MicroK8s-1.19 fails however the rest of them do pass.

@Rishit-dagli
Copy link
Contributor Author

If you are interested in updating the docs too, here is the associated doc: https://github.com/project-akri/akri-docs/blob/main/docs/user-guide/cluster-setup.md

Yep I had created project-akri/akri-docs#52 updating the docs

@Rishit-dagli
Update version
4d07d75 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli Rishit-dagli force-pushed the Rishit-dagli-containerd branch from cb726dc to 4d07d75 Compare September 7, 2022 15:12
@adithyaj adithyaj mentioned this pull request Sep 7, 2022
Merged
2 tasks
kate-goldenring
kate-goldenring approved these changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

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

This looks great! Once we remove EOL k8s versions from the matrix per #506 (comment), i think this will be good to go. If you want to speed this along, feel free to remove k8s 1.16 from the matrix.

@Rishit-dagli
Remove Kubernetes 1.16
9212456 
Signed-off-by: Rishit Dagli <[email protected]>
@Rishit-dagli
Copy link
Contributor Author

This looks great! Once we remove EOL k8s versions from the matrix per #506 (comment), i think this will be good to go. If you want to speed this along, feel free to remove k8s 1.16 from the matrix.

Gotcha, I pushed 9212456 which does that

@adithyaj adithyaj mentioned this pull request Sep 8, 2022
Closed
@kate-goldenring kate-goldenring merged commit 3476dc8 into project-akri:main Sep 8, 2022
@Rishit-dagli Rishit-dagli deleted the Rishit-dagli-containerd branch September 9, 2022 02:03
leoluKL pushed a commit to leoluKL/akri that referenced this pull request Sep 23, 2022
@Rishit-dagli
Use containerd as default container runtime socket for standard Kub…
d36dda4 
…ernetes (project-akri#508)

* Update tests

Signed-off-by: Rishit Dagli <[email protected]>

* Update helm templates

Signed-off-by: Rishit Dagli <[email protected]>

* Update helm values

Signed-off-by: Rishit Dagli <[email protected]>

* Update helm lint commands

Signed-off-by: Rishit Dagli <[email protected]>

* Updates tests

Signed-off-by: Rishit Dagli <[email protected]>

* Update version

Signed-off-by: Rishit Dagli <[email protected]>

* Remove Kubernetes 1.16

Signed-off-by: Rishit Dagli <[email protected]>

Signed-off-by: Rishit Dagli <[email protected]>
Signed-off-by: Leo Lu <[email protected]>
@adithyaj adithyaj mentioned this pull request Sep 27, 2022
Merged
8 tasks
adithyaj added a commit that referenced this pull request Oct 19, 2022
@adithyaj @kate-goldenring
@Rishit-dagli @karok2m @github-actions
Rust toolchain update to 1.63.0 (#507)
d1f7571 
* Use latest version of clap in webhook to remove unmaintained ansi_term dep

Signed-off-by: Kate Goldenring <[email protected]>

* Bump version

Signed-off-by: Kate Goldenring <[email protected]>

* update toml files to v1.63.0 (latest)

Signed-off-by: Adithya J <[email protected]>

* update rust version + use env var in command

Signed-off-by: Adithya J <[email protected]>

* update toolchain + actions

Signed-off-by: Adithya J <[email protected]>

* update readme with new latest version

Signed-off-by: Adithya J <[email protected]>

* revert tarpaulin to 0.20.1, 0.21.0 is not on docker hub yet

Signed-off-by: Adithya J <[email protected]>

* Use `containerd` as default container runtime socket for standard Kubernetes (#508)

* Update tests

Signed-off-by: Rishit Dagli <[email protected]>

* Update helm templates

Signed-off-by: Rishit Dagli <[email protected]>

* Update helm values

Signed-off-by: Rishit Dagli <[email protected]>

* Update helm lint commands

Signed-off-by: Rishit Dagli <[email protected]>

* Updates tests

Signed-off-by: Rishit Dagli <[email protected]>

* Update version

Signed-off-by: Rishit Dagli <[email protected]>

* Remove Kubernetes 1.16

Signed-off-by: Rishit Dagli <[email protected]>

Signed-off-by: Rishit Dagli <[email protected]>

* bump version

Signed-off-by: Kate Goldenring <[email protected]>

* A workflow to auto bump versions (#510)

* A workflow to auto update versions

Signed-off-by: Rishit Dagli <[email protected]>

* Don't run on comment delete

Signed-off-by: Rishit Dagli <[email protected]>

* Allow to checkout on a fork

Signed-off-by: Rishit Dagli <[email protected]>

* Update PR template (#511)

Signed-off-by: Rishit Dagli <[email protected]>

Signed-off-by: Rishit Dagli <[email protected]>

* kubernetes version update

Signed-off-by: Adithya J <[email protected]>

* adding self as a code maintainer

Signed-off-by: Adithya J <[email protected]>

* Add cri-socket arg, get logs for k8s

Signed-off-by: Adithya Jayachandran <[email protected]>

* add images to CTR for containerd

Signed-off-by: Adithya Jayachandran <[email protected]>

* Add status on fail (best effort)

Signed-off-by: Adithya Jayachandran <[email protected]>

* taint appropriate labels

Signed-off-by: Adithya Jayachandran <[email protected]>

* Helm: Enabling passing multi-line strings for custom.configuration.discoveryDetails parameter in Helm values. (#522)

* Enabling passing multi-line strings for custom.configuration.discoveryDetails.

Signed-off-by: Karapet Kostandyan <[email protected]>

* Helm: Enabled usage of network connection for custom discovery handler. (#523)

* Helm: Enabled usage of network connection for custom discovery handler.

Signed-off-by: Karapet Kostandyan <[email protected]>

* Update k3s, microk8s

Signed-off-by: Adithya Jayachandran <[email protected]>

* Update patch version

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Update patch version

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix borrow deref ref and use as_ref instead of to_string as linter specifies

Signed-off-by: Adithya Jayachandran <[email protected]>

* needless match lint

Signed-off-by: Adithya Jayachandran <[email protected]>

* Rust format update + ignoring derive PartialEq and Eq check.
prost has an issue where it does not derive Eq along with PartialEq.
Bypassing this until this is solved

Signed-off-by: Adithya Jayachandran <[email protected]>

* invalid ip address msg now reworded to invalid socket address
updating check for that

Signed-off-by: Adithya Jayachandran <[email protected]>

* update opcua to 0.11.0 to fix out of bounds write + uncontrolled resource consumption

Signed-off-by: Adithya J <[email protected]>

* Cargo.lock was previously pushed, updated with latest dependencies

Signed-off-by: Adithya J <[email protected]>

* Update to version 0.8.22

Signed-off-by: Adithya Jayachandran <[email protected]>

* update toml files to v1.63.0 (latest)

Signed-off-by: Adithya J <[email protected]>

* update rust version + use env var in command

Signed-off-by: Adithya J <[email protected]>

* update toolchain + actions

Signed-off-by: Adithya J <[email protected]>

* update readme with new latest version

Signed-off-by: Adithya J <[email protected]>

* revert tarpaulin to 0.20.1, 0.21.0 is not on docker hub yet

Signed-off-by: Adithya J <[email protected]>

* fix borrow deref ref and use as_ref instead of to_string as linter specifies

Signed-off-by: Adithya Jayachandran <[email protected]>

* needless match lint

Signed-off-by: Adithya Jayachandran <[email protected]>

* Rust format update + ignoring derive PartialEq and Eq check.
prost has an issue where it does not derive Eq along with PartialEq.
Bypassing this until this is solved

Signed-off-by: Adithya Jayachandran <[email protected]>

* invalid ip address msg now reworded to invalid socket address
updating check for that

Signed-off-by: Adithya Jayachandran <[email protected]>

* version 0.8.23

Signed-off-by: Adithya Jayachandran <[email protected]>

Signed-off-by: Kate Goldenring <[email protected]>
Signed-off-by: Adithya J <[email protected]>
Signed-off-by: Rishit Dagli <[email protected]>
Signed-off-by: Adithya J <[email protected]>
Signed-off-by: Adithya Jayachandran <[email protected]>
Signed-off-by: Karapet Kostandyan <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Kate Goldenring <[email protected]>
Co-authored-by: Adithya J <[email protected]>
Co-authored-by: Rishit Dagli <[email protected]>
Co-authored-by: Karapet Kostandyan <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kate-goldenring kate-goldenring kate-goldenring approved these changes

@bfjelds bfjelds Awaiting requested review from bfjelds

@jiria jiria Awaiting requested review from jiria

@Britel Britel Awaiting requested review from Britel

@romoh romoh Awaiting requested review from romoh

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

Successfully merging this pull request may close these issues.

Switch to using containerd as default container runtime socket for standard Kubernetes
2 participants
@Rishit-dagli @kate-goldenring