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

Update Kubernetes driver #1667

Merged
merged 53 commits into from May 18, 2022
Merged

Update Kubernetes driver #1667

merged 53 commits into from May 18, 2022

Conversation

dimgal1
Copy link
Contributor

@dimgal1 dimgal1 commented Mar 17, 2022

Updates for Kubernetes driver

Description

Implement list methods for Kubernetes nodes,services,deployments and node/pod metrics.
Add more fields to pods & containers.
Rename clusters to namespaces since cluster was used as an alias for namespace.
Add type hints

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2022

Codecov Report

Merging #1667 (5232216) into trunk (b1e4005) will decrease coverage by 0.01%.
The diff coverage is 78.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk    #1667      +/-   ##
==========================================
- Coverage   83.27%   83.25%   -0.02%     
==========================================
  Files         400      400              
  Lines       87227    87463     +236     
  Branches     9260     9294      +34     
==========================================
+ Hits        72638    72819     +181     
- Misses      11448    11488      +40     
- Partials     3141     3156      +15     
Impacted Files Coverage Δ
libcloud/container/drivers/kubernetes.py 68.75% <67.20%> (-6.56%) ⬇️
libcloud/container/base.py 89.23% <100.00%> (+0.16%) ⬆️
libcloud/test/container/test_kubernetes.py 98.52% <100.00%> (+1.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1e4005...5232216. Read the comment docs.

@@ -45,6 +45,7 @@ def __init__(
ip_addresses, # type: List[str]
driver, # type: ContainerDriver
extra=None, # type: dict
created_at=None, # type: str
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think it should be fine adding it to the base API since it's added to the end and shouldn't break existing code.

created_at: str,
replicas: int,
selector: Dict[str, Any],
extra: Optional[Dict[str, Any]] = None,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for including type annotations everywhere 👍

)


def to_n_bytes(memory_str: str) -> int:
Copy link
Member

Choose a reason for hiding this comment

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

It also wouldn't hurt to add direct test cases for all those utility conversion functions :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay, I added the test cases

Copy link
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

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

Sorry for the delay - thanks for addressing the feedback and LGTM 👍

@Kami Kami merged commit f906685 into apache:trunk May 18, 2022
@Kami Kami added this to the v3.6.0 milestone Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants