API is not correctly setting the cluster's volume profile flag while import flow

[gnehapk]

UI fired import API call with request data as "Cluster.volume_profiling_flag: enable", in the etcd instance its set to leave-as-is instead of enable.

"value": "{\"status\": \"new\", \"username\": \"admin\", \"run\": \"tendrl.flows.ImportCluster\", \"job_id\": \"278d1597-7967-42b6-9e68-4df163eb0a2d\", \"parameters\": {\"TendrlContext.integration_id\": \"b2bfaaf3-8c69-4aa2-8f1d-14191f469b18\", \"Cluster.volume_profiling_flag\": \"leave-as-is\"}, \"tags\": [\"tendrl/integration/b2bfaaf3-8c69-4aa2-8f1d-14191f469b18\"], \"created_at\": \"2018-03-14T10:01:22Z\", \"created_from\": \"API\", \"type\": \"node\", \"name\": \"ImportCluster\"}",

[shirshendu]

@gnehapk Could you please provide the curl command of the API call made? Better to document the faulty call within this issue.

[gnehapk]

@shirshendu

[shirshendu]

@gnehapk The UI is not making the API call correctly.
The API doc mentions the following:

UI is sending Cluster.volume_profiling_flag.

[gnehapk]

@shirshendu As per @shtripat comment the request data contains Cluster.volume_profiling_flag property. Didn't receive any notification for this change.

I have created a UI issue Tendrl/ui#871 for it. You can close this issue.

[r0h4n]

The flag should be "Cluster.volume_profiling_flag", @shirshendu the api doc needs to be updated.

every attribute/param in Tendrl is bound to a object, in this case it is the "Cluster" object

Check: https://github.com/Tendrl/commons/blob/master/tendrl/commons/objects/cluster/__init__.py#L24

@gnehapk you dont need to change how the UI calls this api.

[shirshendu]

Got it. Seems like these changes were made in gluster-integration, and not percolated up to api. Taking it up.

[shirshendu]

Wait, I don't agree with this. The API is already functioning just fine. It is already a clusters/:id/import resource call, so the context is already set. On sending the volume_profiling_flag as mentioned in the API doc, we set the correct object in the job, i.e., we specify 'Cluster.volume_profiling_flag`.
See: https://github.com/Tendrl/api/blob/master/app/controllers/clusters_controller.rb#L66

I'm assuming this is obviously required at the job level, because different objects make use of the job queue.
However, I do not see any benefit in bleeding that verbosity into the API. Thoughts?

[r0h4n]

You are correct that Import api works with volume_profiling_flag.

tendrl-api was started with aims to make it a self discovering api which uses the definitions for flows and objects and presents them as api calls. This is why I want even the API to adhere and use proper definitions while accepting http params and providing http response. Hence need to use Cluster.volume_profiling_flag.

We should also make similar changes across API where orphan attributes or params are found

Thoughts?

[shirshendu]

Hmm, I agree about orphan attributes; In this particular scenario, Cluster.volume_profiling_flag is an orphan attribute, because it is actually an argument to a job, and yet being given in a cluster API.
I think it would make more sense to have a clusters/:id/import API which takes a param called job_opts. In my opinion, it makes intuitive sense in the context of this API, that these are inputs to the import job.
POST clusters/:id/import

`"job_opts": { "Cluster.volume_profiling_flag": "enable" }

This way, tendrl-api can get out of the way, and simply forward the given options to the job.

[shirshendu]

Documenting discussion with @r0h4n here:
The philosophy we want to follow for the API is that all non-GET calls are asynchronous, and will be queueing jobs; and GET calls would just pick up data from etcd.

So Tendrl-api should be forwarding the params coming in from the UI to the job as-is, and instead of per-API validation, we need more generic validation that can work on the tendrl definitions.