tmforum-oda / oda-canvas

Apache License 2.0
19 stars 49 forks source link

operator: DependentAPI name is prefixed with dapi- #302

Closed LesterThomas closed 1 month ago

LesterThomas commented 1 month ago

Description

This is a small issue with the naming of the DependentAPI created from the component. For the ExposedAPI, the name of the ExposedAPI is <release>-<componentname>-<exposedapiname>

For the DependentAPI, it has an additional prefix of dapi which I think is unnecessary and doesn't follow the pattern (it is unnecessary since we know it is of Kind DependentAPI).

ferenc-hechler commented 1 month ago

I will start working on this issue, as soon as the PR #294 (automate docker build) is merged, because the changes require an updated docker image.

ferenc-hechler commented 1 month ago

For testing I deployed the productcatalog with dependencies in the testData. As a release name I chose "dapitest". The deployment failed with an error:

$ helm install dapitest -n components productcatalog-dependendent-API-v1beta3

  Release "dapitest" does not exist. Installing it now.
  Error: 2 errors occurred:
        * Service "dapitest-mongodb" is invalid: spec.ports[0].targetPort: Invalid value: "dapitest-mongodb": must be no more than 15 characters
        * Deployment.apps "dapitest-mongodb" is invalid: spec.template.spec.containers[0].ports[0].name: Invalid value: "dapitest-mongodb": must be no more than 15 characters

This means our release name for the productcatalog is limited to 7 characters, which is quite limited. I will create an issue to find a way to increase the allowed release name length.

ferenc-hechler commented 1 month ago

I deployed the productcatalog component with a shorter release name "dapit" and now the deployment was successful. Looking at the generated custom resource:

$ kubectl get dependentapis -n components

  NAME                                                           DEPAPI_ENDPOINT                                                                                            DEPAPI_READY
  dapit-productcatalogmanagement-dapi-downstreamproductcatalog   http://components.ihc-dt.cluster-3.de/alice-productcatalogmanagement/tmf-api/productCatalogManagement/v4   true

So, "dapi" it is not a prefix it is an infix to separate the component-name from the dependent-api name.

dapit-productcatalogmanagement-dapi-downstreamproductcatalog
<release>-<component>-dapi-<dependencyname>

It was thought to avoid name-clashes if \<component> and \<dependencyname> also contain "-" characters.

I must admit this is not very likely, but it is possible to create name clashes on purpose. With normal naming rules it should not happen.

One example for a name-clash:

release component dependencyname custom resource name
abcrel comp1-summer party abcrel-comp1-summer-party
abcrel comp1 summer-party abcrel-comp1-summer-party
ferenc-hechler commented 1 month ago

The relevant piece of code:

https://github.com/tmforum-oda/oda-canvas/blob/82091fa04ed383296861a7f0c37f198cf57487da/source/operators/componentOperator/componentOperator.py#L448

Changing this to dapi_base_name = name might fix the issue. But we have to check if there exist other dependencies to the naming convention.

ferenc-hechler commented 1 month ago

This issue is a perfect example, how the new Docker build process works:

https://github.com/tmforum-oda/oda-canvas/pull/305/files

image

Step Description
branch create a branch, naming has to follow "feature/..." or "odaa-..." otherwise the automated docker builds are not active
1 Increment the version number of the dockerfile which will be updated and and set the prereleaseSuffix, e.g to the issue number
2 change the source code. This triggers the docker build of the prerelease version
3 adjust the tests to cover the changed code (steps 2 and 3 might iterate multiple times)
4 prepare everything for the Pull-Request. Set chart-version numbers, document changes, also update the version number in the dependent charts
PR create Pull-Request -> BDD Tests are executed and should be successfull
5 before merging, remove all prereleaseSuffixes. This will make the "no-prerelease-suffixes"-check in the PR green
Merge the merge into the master branch triggers the build of the release docker images