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

Fix kubernetes cloud deployment issues #8447

Merged
merged 19 commits into from
Feb 7, 2024
Merged

Fix kubernetes cloud deployment issues #8447

merged 19 commits into from
Feb 7, 2024

Conversation

yashgorana
Copy link
Contributor

@yashgorana yashgorana commented Feb 2, 2024

Description

Closes https://github.com/OpenMined/Heartbeat/issues/964, https://github.com/OpenMined/Heartbeat/issues/966, https://github.com/OpenMined/Heartbeat/issues/971, https://github.com/OpenMined/Heartbeat/issues/968

This change fixes PVC MultiAttach error with the following fixes:

  • credentials-data volume is now a Secret that is mounted to all the worker pods. This way credentials-data will become ReadWriteOnce mounted only by the backend pod.
  • worker-builds PVC is removed and internal registry is added as a cache. Build always pushes to internal registry. Push copies it from internal to remote registry.

Additionally refactored core kubernetes helper functions

Affected Dependencies

List any dependencies that are required for this change.

How has this been tested?

  • Describe the tests that you ran to verify your changes.
  • Provide instructions so we can reproduce.
  • List any relevant details for your test configuration.

Checklist

@yashgorana yashgorana added Type: Bug 🐛 Some functionality not working in the codebase as intended 0.8.4 labels Feb 2, 2024
@yashgorana yashgorana self-assigned this Feb 2, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@yashgorana yashgorana removed the Type: Bug 🐛 Some functionality not working in the codebase as intended label Feb 2, 2024
scripts/build_images.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@rasswanth-s rasswanth-s left a comment

Choose a reason for hiding this comment

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

Stellar Work @yashgorana 💯

Copy link
Member

@shubham3121 shubham3121 left a comment

Choose a reason for hiding this comment

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

looks good, just minor comments.


class ImageUtils:
@staticmethod
def parse_tag(tag: str) -> Tuple[Optional[str], str, str]:
Copy link
Member

Choose a reason for hiding this comment

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

NITPICK: I think we can make them as independent functions, instead of static methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a way to group similarly related functions in a namespace in this case "ImageUtils", so i get all the functions in one import. Also keeps the code a bit neater. Is it a much-needed change?

packages/syft/src/syft/service/worker/utils.py Outdated Show resolved Hide resolved
@rasswanth-s rasswanth-s merged commit 2431411 into dev Feb 7, 2024
31 of 32 checks passed
@rasswanth-s rasswanth-s deleted the yash/infra-fixes branch February 7, 2024 03:37
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

3 participants