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

chore: add dataprovider charts #53

Merged
merged 38 commits into from
Apr 22, 2024
Merged

Conversation

ds-jhartmann
Copy link
Contributor

@ds-jhartmann ds-jhartmann commented Feb 28, 2024

Description

  • Added tx-data-provider Helm Chart including Charts for
    • tractusx-connector:0.5.3
    • postgresql:12.12.10 for EDC
    • digital-twin-registry:0.4.5
    • simple-data-backend
    • Test data seeding
  • Added data provider Chart to the umbrella Chart as dataconsumer and dataprovider examples including the necessary configuration and name overrides

Relates to eclipse-tractusx/item-relationship-service#375

Not included

  • MIW, Vault and the configuration of these services in EDC

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@ds-jhartmann ds-jhartmann marked this pull request as draft February 28, 2024 15:10
fullnameOverride:

image:
repository: ghcr.io/catenax-ng/catenax-at-home/provider-backend-service
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there nothing equivalent in Eclipse Tractus-X.
If no, we should move this asap. The catenax-ng org will potentially be gone.
The repo itself states, that it is out of date. There is also no 3rd party dependency check, which we would need to do for it.
So in my opinion, we cannot use this repo as dependency here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree and tried to find something similar but it looks like this was not migrated to Eclipse Tractus-X.
The "proper" way would probably be to use one of the AASX Submodel Servers mentioned in the Digital Twin Kit
Since none of these Projects provide their own Helm Charts we cannot add them easily and I'm not familiar with the APIs and how to use them or integrate them with the existing test data upload script we were using in the IRS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok understood,

@stephanbcbauer, @danielmiehle, @Siegfriedk, @giterrific: What is your view from a project lead perspective?
Do we need a new repo/product, to pick up these missing things. I think we definitely need that for a proper way forward.

@AngelikaWittek: Do you see anything, that would prevent us from using the service provided by catenax-ng/catenax-at-home. I guess it as least proper IP checks right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SebastianBezold is it about the image itself? We could fork the repo (no clue if this is legally okay for EF), we could clone the image into ours (to have it for test purposes only), we could fork the docker file and build it ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the insights. Since we only need a small docker image with create and get capabilities, we decided to implement this ourselves. As discussed with @jzbmw I prepared a PR to add this to this repo: #54
I will remove the submodelservers chart form this branch. It will be added in scope of the other PR

@ds-jhartmann
Copy link
Contributor Author

Installing seems to be failing due to
0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod..

@ds-jhartmann ds-jhartmann marked this pull request as ready for review April 5, 2024 08:36
@Siegfriedk
Copy link
Contributor

Installing seems to be failing due to 0/1 nodes are available: 1 Insufficient cpu. preemption: 0/1 nodes are available: 1 No preemption victims found for incoming pod..

Doesn't seem like we have any finetuning going on. You can always finetune the CPU requests. Independent of it, we can also use large runners.

evegufy added a commit that referenced this pull request Apr 16, 2024
change to `helm install` instead of `ct install` to don't run into that error anymore because when using helm install (also on local Minikube) the chart (for instance for this PR #53) installs without error.
also: add an informational step to describe the node for more awareness about available resources in the future and upgrade some actions.
Copy link
Contributor

@evegufy evegufy 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 in general 👍 just essentially the same point as already for the simple-data-backend: we need to be able to test the tx-data-provider via workflow, see #64, otherwise it's to hard to maintain and currently the install fails https://github.com/eclipse-tractusx/tractus-x-umbrella/actions/runs/8705281375/job/23875363023 (also on my local), could you please look into that?

…a-provider

chore: add tx-data-provider to helm checks
@almadigabor
Copy link
Contributor

almadigabor commented Apr 18, 2024

Hey @ds-jhartmann! The controlplane and dataplane pods are throwing NullPointerExceptions here and here that should be investigated.

@ds-jhartmann
Copy link
Contributor Author

Hey @ds-jhartmann! The controlplane and dataplane pods are throwing NullPointerExceptions here and here that should be investigated.

Thanks for the hint. I already saw this and I'm working on moving the vault to the tx-data-provider chart. EDC required a vault connection to start

Copy link
Contributor

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

LGTM

@ds-jhartmann ds-jhartmann merged commit 67e69c2 into main Apr 22, 2024
4 checks passed
@evegufy evegufy deleted the chore/add-dataprovider-charts branch August 13, 2024 18:32
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.

5 participants