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 Helm Chart to add more flexibility #4

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

manfuin
Copy link
Contributor

@manfuin manfuin commented Nov 13, 2024

When deploying for my use-case I have refactored couple of places in the chart:

  • OAUTH machinery in explicitly enabled/disabled instead of relying on interlink usage of the socket. It might be IP connectivity without OAUTH but NetworkPolicies or even ServiceMesh instead.
  • DataRootFolder is not just static path but a volume mount definition instead. This offers a flexibility to define whatever volume your infra is required to get it working: hostPath, nfs, PVC, etc.
  • Plugin container fixes (main one: mount plugin config instead of interlink config)
  • More conditionals to avoid mounting unnecessary volumes or defining unnecessary environment if feature is disabled

Hope you can find it useful.

P.S. I didn't bump the chart version in PR.

@manfuin manfuin requested a review from dciangot as a code owner November 13, 2024 13:26
Copy link
Collaborator

@dciangot dciangot left a comment

Choose a reason for hiding this comment

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

@manfuin thanks for this PR, I agree with your points.

Can you tell which scenario did you test? So I can give a quick spin to the others and checking that they still working?

@dciangot dciangot added the enhancement New feature or request label Nov 13, 2024
@dciangot
Copy link
Collaborator

ok, If I recall it correctly should be the edge case the one you tested.

Let me try the in-cluster and tunneled oene. If everything still works I'll merge the PR

@dciangot dciangot self-requested a review December 3, 2024 18:40
Copy link
Collaborator

@dciangot dciangot left a comment

Choose a reason for hiding this comment

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

LGTM. sorry for the delay @manfuin , I rechecked all the current deployment I could replicate, and looks like good go.

@dciangot dciangot merged commit 1ce5f06 into interTwin-eu:main Dec 3, 2024
1 of 2 checks passed
@manfuin
Copy link
Contributor Author

manfuin commented Dec 5, 2024

Thanks @dciangot! Sorry I had overlooked all previous github emails in my mailbox. It was actually both in-cluster and edge I was playing with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants