Initialize EAM /appinstances callback

[travishepworth]

What type of PR is this?

Add one of the following kinds:

• enhancement/feature

What this PR does / why we need it:

This PR adds a callback definition for POST /appinstances to return that status of the application after the instantiation is accepted. This allows clients to receive an asynchronous status update for their application.

Which issue(s) this PR fixes:

Fixes #370

Special notes for reviewers:

This PR only adds the callback definition. Implementation is backward-compatible.

Changelog input

 release-note
 Add callback response support to the EAM POST /appinstances endpoint to return application status after instantiation

Additional documentation

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: travishepworth / name: travmonkey (a6ec5ea, d439fdc)

[jgarciatovar]

Hi @travishepworth ,

Some comments from my side. According to Commonalities (https://github.com/camaraproject/Commonalities/blob/main/documentation/CAMARA-API-Event-Subscription-and-Notification-Guide.md), the expected response code for notifications is 204 instead of 202.

In addition to that, some errors are also mandatory to be included on any notifications. The Following Error codes must be present:

• for POST: 400, 401, 403, 410, 429

I recommend that we use the template available at Commonalities repo: https://github.com/camaraproject/Commonalities/blob/main/artifacts/camara-cloudevents/event-subscription-template.yaml

[travishepworth]

Cool, I can start looking at implementing based off the template and the missing codes.

[jgarciatovar]

hi @travishepworth - note that #356 PR might have some impact on this one. I would wait for /deployments to be added before adding any callback notifications. That way, we could also use this PR to define callback notifications for a deployment.

Once the deployment is added, we could check details on which callback notifications could be added there. I guess that we should also add notifications per application instance managed by such deployment.

[travishepworth]

Gotcha, I'll keep this on hold until I see #356 get merged.

[jgarciatovar]

@gunjald / @travishepworth - #356 has been already merged. Now, I think that it is time to add here on this PR the same for the deployment method. In the end, the deployment method should also include the same callback interface that informs the client about application instance changes/updates for any of the instances included in a deployment.

What do you think?

[travishepworth]

Sounds great @jgarciatovar , I can start drafting the next iteration and update the PR accordingly.

[travishepworth]

The PR has now been updated to include deployments, and aligned with the commonalities.

I added in all the required objects to setup any of the streaming subscriptions as seen at: https://github.com/camaraproject/Commonalities/blob/main/artifacts/camara-cloudevents/event-subscription-template.yaml

However, this is likely undesirable, should another endpoint be implemented specifically for subscriptions?

Also note that there are still default descriptions in some of the objects. I will wait until it has been decided whether this stays as a callback to the POST endpoints, or moved to another endpoint (as well as until it has been determined which subscription methods should be made available)

[JoseMConde]

Hi @travishepworth , sorry for the delay in the feedback.
I have been checking commonalities and trying to find the best solution, it is not easy.
We have two different options of supscriptions:https://github.com/camaraproject/Commonalities/blob/main/documentation/CAMARA-API-Event-Subscription-and-Notification-Guide.md

• Instance-based subscription (implicit creation)

• Resource-based subscription (explicit creation)

Each of them has different implications, in my view, for this PR we can let the callback stay in the POST and make the minimum modifications to be aligned with commonalities (probably we don't need the rest of the messaging protocols right now). I think it's worth to have the discussion of the the new subscription endpoint, but perhaps it's better to move it to a different PR.

@camaraproject/edge-cloud_codeowners WDYT??

[JoseMConde]

M1 commonalities issue Issue. Could this help us??

[JoseMConde]

@camaraproject/edge-cloud_codeowners please, could you review the last changes??

[JoseMConde]

A couple of doubts @travishepworth

[gunjald]

I think "status" parameter should have well-defined instance state enum values to let the API client understand the app instance running state. It could be e.g., pending, creating, running, failed, terminating etc with each value well defined and gives clear indication on instance health.

Also, how the deployment status change and instance status change events are to be differentiated as there is a single "status" field used for deployment and instance state change. Or I am missing something?

[travishepworth]

The deployment status change should be a different object, agreed. I believe a list would be a better return object.

[travishepworth]

Updated to an enum for the status, and for the deployments it is now set to an array instead of a single object

[JoseMConde]

LGTM

[clundie-CL]

@JoseMConde @Kevsy @gunjald Please see PR #407. It is a continuation of this PR and explains why I opened it instead of picking this one up.

[gunjald]

Changes seems fine to me.