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

[TDX] Added Intel TDX support to helm charts #799

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

JakubLedworowski
Copy link

@JakubLedworowski JakubLedworowski commented Feb 11, 2025

Description

Confidential computing in AI in the cloud focuses on protecting sensitive data and computations from unauthorized access and tampering. It uses advanced security technologies, such as hardware-based isolation and encryption, to create secure environments where data and AI models can be processed safely. This ensures that even cloud service providers cannot access the data, providing a higher level of privacy and security. By leveraging confidential computing, organizations can confidently use AI in the cloud for tasks that involve sensitive information, such as healthcare data analysis or financial predictions, while complying with strict data protection regulations.

Subcharts enabled to be run with Intel TDX:

  • embedding-usvc
  • guardrails-usvc
  • llm-uservice
  • redis-vector-db
  • reranking-usvc
  • retriever-usvc
  • tei
  • teirerank
  • tgi
  • vllm
  • ui
  • data-prep
  • nginx

Issues

List the issue or RFC link this PR is working on. If there is no such link, please mark it as n/a.

Type of change

List the type of change like below. Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would break existing design and interface)

Dependencies

List the newly introduced 3rd party dependency if exists.

Tests

Tested the generated templates with and without Intel TDX.
Tested the deployment of chatqna example on the running cluster.

helm-charts/TDX.md Outdated Show resolved Hide resolved
helm-charts/TDX.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@lianhao lianhao left a comment

Choose a reason for hiding this comment

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

We should combine the common-library and tdx charts into a single tdx library chart. The benefit of using library chart can be found at here

helm-charts/common/common-library/Chart.yaml Outdated Show resolved Hide resolved
helm-charts/TDX.md Show resolved Hide resolved
helm-charts/TDX.md Show resolved Hide resolved
helm-charts/TDX.md Outdated Show resolved Hide resolved
Copy link

@ksandowi ksandowi left a comment

Choose a reason for hiding this comment

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

After validation all services marked to be run with TDX works fine. E2E chatqna responses successfully

Copy link

@ksandowi ksandowi left a comment

Choose a reason for hiding this comment

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

Update helm install with following minimal resources required for TDX

helm-charts/TDX.md Show resolved Hide resolved
@kding1 kding1 requested review from hshen14 and ftian1 February 13, 2025 00:49
helm-charts/common/tdx/values.yaml Outdated Show resolved Hide resolved
helm-charts/common/tei/templates/deployment.yaml Outdated Show resolved Hide resolved
helm-charts/common/common-library/Chart.yaml Outdated Show resolved Hide resolved
@JakubLedworowski
Copy link
Author

We should combine the common-library and tdx charts into a single tdx library chart. The benefit of using library chart can be found at here

Hi @lianhao, I tried to do it with a single library at the beginning, but the problem I faced is that there is a common logic for including podAnnotations. With annotations for Intel TDX we have to merge with the existing solution. That is why I introduced a common-library, which is always included and inside it checks if tdx values are present.

If you have an idea how to make it a single library I am open to suggestions :)

@lianhao lianhao force-pushed the jledworo/enable-intel-tdx branch from d8be139 to a81a790 Compare February 13, 2025 09:43
@JakubLedworowski
Copy link
Author

We should combine the common-library and tdx charts into a single tdx library chart. The benefit of using library chart can be found at here

Hi @lianhao, I tried to do it with a single library at the beginning, but the problem I faced is that there is a common logic for including podAnnotations. With annotations for Intel TDX we have to merge with the existing solution. That is why I introduced a common-library, which is always included and inside it checks if tdx values are present.

If you have an idea how to make it a single library I am open to suggestions :)

After a call we simplified the solution into a single library included as permanent dependency.
A common part with annotations is always included but the tdx part is only appended when tdxEnabled is true.

Copy link
Collaborator

@lianhao lianhao left a comment

Choose a reason for hiding this comment

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

Please fix the DCO issue. Others seems fine to me

JakubLedworowski and others added 12 commits February 14, 2025 11:01
Subcharts:
 - embedding-usvc
 - guardrails-usvc
 - llm-uservice
 - redis-vector-db
 - reranking-usvc
 - retriever-usvc
 - tei
 - teirerank
 - tgi
 - vllm

Signed-off-by: Jakub Ledworowski <[email protected]>
for more information, see https://pre-commit.ci

Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Subcharts:
- ui
- data-prep
- nginx

Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
This reverts commit 0031ce3.

Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
Signed-off-by: Jakub Ledworowski <[email protected]>
@JakubLedworowski JakubLedworowski force-pushed the jledworo/enable-intel-tdx branch from 3591d6c to f3031be Compare February 14, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants