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

Lazily initialize kubernetes client only when using kube api watcher #56

Merged
merged 3 commits into from
Apr 6, 2023

Conversation

jeevb
Copy link
Contributor

@jeevb jeevb commented Apr 6, 2023

TL;DR

Lazily load k8s client only in kubeapi watcher. This prevents init container errors when running with service accounts that have automountServiceAccountToken: false set. Smoke tested in sandbox with and without automountServiceAccountToken: false, with both the default shared process namespace watcher and kubeapi watcher respectively.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Moved initialization of k8s client from copilot root command to NewKubeAPIWatcher function. Now it is only initialized when the kube-api watcher is specified.

@welcome
Copy link

welcome bot commented Apr 6, 2023

Thank you for opening this pull request! 🙌
These tips will help get your PR across the finish line: - Most of the repos have a PR template; if not, fill it out to the best of your knowledge. - Sign off your commits (Reference: DCO Guide).

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
@jeevb jeevb force-pushed the jeev/lazy-k8s-client branch from e959f4b to a254cf2 Compare April 6, 2023 20:22
@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

Merging #56 (08bbbb5) into master (97772fd) will increase coverage by 0.91%.
The diff coverage is n/a.

❗ Current head 08bbbb5 differs from pull request most recent head 8cc786e. Consider uploading reports for the commit 8cc786e to get more accurate results

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
+ Coverage   16.87%   17.79%   +0.91%     
==========================================
  Files           3        3              
  Lines         474      399      -75     
==========================================
- Hits           80       71       -9     
+ Misses        381      315      -66     
  Partials       13       13              
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
@jeevb jeevb marked this pull request as ready for review April 6, 2023 21:02
@jeevb jeevb requested review from EngHabu and kumare3 as code owners April 6, 2023 21:02
Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
@jeevb jeevb merged commit 12f9ff5 into master Apr 6, 2023
@welcome
Copy link

welcome bot commented Apr 6, 2023

Congrats on merging your first pull request! 🎉

@jeevb jeevb deleted the jeev/lazy-k8s-client branch April 6, 2023 21:37
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
…56)

* Lazily initialize kubernetes client only when using kube api watcher

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>

* Fix parsing of podname to watch from CLI args

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>

* Minor fix

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>

---------

Signed-off-by: Jeev B <jeevb@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants