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.

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

Worker nodes should use containerd vs. Docker as default CRI #171

Closed
eak13 opened this issue Jun 7, 2021 · 11 comments
Closed

Worker nodes should use containerd vs. Docker as default CRI #171

eak13 opened this issue Jun 7, 2021 · 11 comments
Assignees
Labels
2-Manifests Relates to manifest/document set related issues enhancement New feature or request priority/critical Items critical to be implemented, usually by the next release ready for review Patchset(s) related to this issue are ready to be reviewed size m 2-5 days [moderate complexity, generic code, or enhancement to existing feature]]
Milestone

Comments

@eak13
Copy link

eak13 commented Jun 7, 2021

With airshipit/airshipctl#456 the switch to using containerd as the default CRI was made. While the control plane nodes deployed in Treasuremap appear to have inherited this change, it appears that the worker nodes are still running Docker by default.

Task:

  1. Make any changes to the worker node deployments for both airship-core & multi-tenant type to ensure they implement containerd vs. Docker as the CRI
  2. Validate the worker nodes actually deploy using containerd.
  3. (Needs discussion) Throw an error if Docker is specified as CRI (it's being deprecated in an upcoming release)
@eak13 eak13 added enhancement New feature or request triage labels Jun 7, 2021
@jezogwza jezogwza added priority/critical Items critical to be implemented, usually by the next release and removed triage labels Jun 9, 2021
@jezogwza jezogwza added this to the v2.1 milestone Jun 9, 2021
@jezogwza jezogwza added the 2-Manifests Relates to manifest/document set related issues label Jun 9, 2021
@matthew-fuller
Copy link
Contributor

Please assign this to me.

@eak13
Copy link
Author

eak13 commented Jun 15, 2021

@matthew-fuller all yours

@matthew-fuller matthew-fuller added the size m 2-5 days [moderate complexity, generic code, or enhancement to existing feature]] label Jun 15, 2021
@matthew-fuller
Copy link
Contributor

After running the deployment gates locally for treasuremap, I checked the runtime version for nodes 01 and 03, and they both say

Container Runtime Version:         containerd://1.4.6

I'll need to verify whether Docker was seen on workers using a different deployment method than I used, and if so, try to replicate the problem and make changes if necessary.

@eak13
Copy link
Author

eak13 commented Jun 16, 2021

@matthew-fuller, if need be, follow up with @aostapenko as he was the one who encountered this during deployment. Thanks.

@aostapenko
Copy link
Contributor

This one is already mostly addressed by https://review.opendev.org/c/airship/treasuremap/+/794678 (merged) and https://review.opendev.org/c/airship/airshipctl/+/796380. Though we still need to switch to using airshipctl workers-capm3 replacements and remove workers-capm3 function from TM

@eak13
Copy link
Author

eak13 commented Jun 16, 2021

@aostapenko I see https://review.opendev.org/c/airship/treasuremap/+/794678 addresses airship-core type, but I didn't see the same for multi-tenant type. Does that still need to be resolved as well?

@aostapenko
Copy link
Contributor

multi-tenant type does not currently define workers on type level. Though virtual-network-cloud site (that uses multi-tenant type) is utilizing airshipctl via vm-infra-bridge/dataplane https://review.opendev.org/c/airship/treasuremap/+/794678/5/manifests/function/vm-infra-bridge/dataplane/kustomization.yaml
On the other hand reference-multi-tenant still does not utilize any function and defines all workers resources on site level. Though I believe this should be addressed separately.

@lb4368 lb4368 modified the milestones: v2.1, v2.2 Jun 16, 2021
@matthew-fuller
Copy link
Contributor

The comments so far (and the fact that I wasn't able to reproduce the issue) seem to indicate that most of the work on this issue has already been addressed by the above patch sets. Removing treasuremap's workers-capm3 function and ensuring all sites and types inherit from airshipctl's instead seems within the scope of this issue, so https://review.opendev.org/c/airship/treasuremap/+/796737 has been created to address this.

@matthew-fuller matthew-fuller added the ready for review Patchset(s) related to this issue are ready to be reviewed label Jun 17, 2021
airshipbot pushed a commit that referenced this issue Sep 13, 2021
- Removes function/workers-capm3 from treasuremap since sites
  and types inherit this function from airshipctl
- Changes virtual-network-cloud site to use airshipctl's
  function/workers-capm3

Relates-To: #171
Change-Id: I8b8a6f32d75c2b1a46ff25906951ff5544055f30
@matthew-fuller
Copy link
Contributor

Change 796737 to remove the workers-capm3 function from treasuremap has been merged, so this issue can likely be closed now. @lb4368 @aostapenko do you agree?

@aostapenko
Copy link
Contributor

+1

@eak13
Copy link
Author

eak13 commented Sep 13, 2021

Closing per above

@eak13 eak13 closed this as completed Sep 13, 2021
@eak13 eak13 modified the milestones: v2.2, v2.1 Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-Manifests Relates to manifest/document set related issues enhancement New feature or request priority/critical Items critical to be implemented, usually by the next release ready for review Patchset(s) related to this issue are ready to be reviewed size m 2-5 days [moderate complexity, generic code, or enhancement to existing feature]]
Projects
None yet
Development

No branches or pull requests

5 participants