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

0.3.0 Charts to not reflect setting changes of the 0.3.0 image #167

Closed
DominikPinsel opened this issue Mar 29, 2023 · 6 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@DominikPinsel
Copy link
Contributor

Describe the bug

As it is now the charts to not reflect the changes from the migration guide.
https://github.com/catenax-ng/tx-tractusx-edc/blob/develop/docs/migration/Version_0.1.x_0.3.x.md

The affected charts are currently from the product-edc repository. But the idea would be to fix them here.

Affected charts:

catenax-ng-product-edc/edc-controlplane --version 0.3.0
catenax-ng-product-edc/edc-dataplane --version 0.3.0
catenax-ng-product-edc/tractusx-connector --version 0.3.0

Context Informations

  • Used version: 0.3.0

Dominik Pinsel [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

@DominikPinsel DominikPinsel added the bug Something isn't working label Mar 29, 2023
@florianrusch-zf
Copy link
Contributor

Are all points not taken into account? Or only a few? If only a few, can we collect them here? That would make the work on the fix easier in the end.

@DominikPinsel
Copy link
Contributor Author

DominikPinsel commented Mar 30, 2023

Are all points not taken into account? Or only a few? If only a few, can we collect them here? That would make the work on the fix easier in the end.

I'm not 100% sure which changes are causing issues in which charts.

I've a feeling the web.http.data to web.http.management change might cause issues in the Control- & Data Plane chart. And the edc.receiver.http.endpoint to edc.receiver.http.dynamic.endpoint might cause issues in the TXDC chart. But haven't tested it myself yet and also cannot say anything about the other changes.

But as we need to update the charts anyway, I decided against investing additional time to test the impact of all the changes.

Dominik Pinsel [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

@tuncaytunc-zf
Copy link
Contributor

tuncaytunc-zf commented Mar 30, 2023

I would say we shouldn't use Control- & Data Plane charts anymore for version newer then 0.1.7 since they are deprecated anyway. Otherwise we have to refactor them to make it compatible with v0.3.x
For the edc.receiver.http.dynamic.endpoint we have to add new property in our TXDC chart.

@ndr-brt
Copy link
Contributor

ndr-brt commented Mar 30, 2023

I would say we shouldn't use Control- & Data Plane charts anymore for version newer then 0.1.7 since they are deprecated anyway. Otherwise we have to refactor them to make it compatible with v0.3.x For the edc.receiver.http.dynamic.endpoint we have to add new property in our TXDC chart.

with hindsight they could have been removed in 0.3.0 as breaking changes were expected anyway. I will create an issue

@DominikPinsel
Copy link
Contributor Author

DominikPinsel commented Mar 30, 2023

So, to sum it up I would suggest to

Suggested Changes

  1. Update the following settings
  • web.http.data
  • web.http.ids
  • web.http.validation
  • web.http.controlplane
  • web.http.dataplane
  • edc.receiver.http.endpoint
  • edc.oauth.public.key.alias
  1. Update the health check endpoints

I'd say all the other changes from the migration documentation are either not related to the chars, additions, or not completely resolved (like the trailing slash in the /public data plane endpoint).

Web Setting Update

Update the following settings

  • web.http.data
  • web.http.ids
  • web.http.validation
  • web.http.controlplane
  • web.http.dataplane

Change the names of the new web endpoints in the following files:

  • Tractus-X Chart
    • values.yaml
    • deployment-controlplane.yaml
    • service-controlplane.yaml
    • ingress-controlplane.yaml
  • ControlPlane Chart
    • values.yaml
    • configmap.yaml
    • deployment.yaml
    • ingress.yaml
    • service.yaml

Other Settings

Update the edc.receiver.http.endpoint and edc.oauth.public.key.alias in the following files:

  • Tractus-X Chart
    • deployment-controlplane.yaml
  • ControlPlane Chart
    • values.yaml

Health Check Update

Change the health check to the management API and add authentication

Affected files:

  • Tractus-X Chart
    • deployment-controlplane.yaml
  • ControlPlane Chart
    • deployment.yaml

TBD: It looks like the data plane health check is not affected? In this case no update necessary.

Documentation Update

  • Do a text search for the outdated settings and update the corresponding documentation
  • Add the changes to the CHANGELOG.MD
  • Write a new migration documentation, as changes in the values.yaml are breaking changes

General Remark
I'd try to avoid changing too much in the values yaml. For example: Even if the new context of the data management API is called management and not data anymore, I'd keep the configured /data path. Other tools may have dependencies to these interfaces and by changing them we make an update of them necessary, too.

Dominik Pinsel [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

@DominikPinsel
Copy link
Contributor Author

I would say we shouldn't use Control- & Data Plane charts anymore for version newer then 0.1.7 since they are deprecated anyway. Otherwise we have to refactor them to make it compatible with v0.3.x For the edc.receiver.http.dynamic.endpoint we have to add new property in our TXDC chart.

Yes, removing these charts will become necessary. I'd not remove them now, but add a disclaimer to the new release that they will not be part of the next release. Like this the stakeholders can plan the work ahead to replace their charts with the new one.

Dominik Pinsel [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

5 participants