fix!(containers): remove ready status

[lusergit]

• :ready and :stopped are redundant statuses, moved to using only :stopped status;
• the check_deployment check when the deployment is available sets the deployment status tio the device deployment status

[noaccOS]

We're also missing the change on deployment_event: while we want to propagate a deployment error in any situation, we want to avoid setting starting/stopping while we're setting up the deployment, otherwise it will mess with the "state machine"

[davidebriani]

Just some nitpicks, but the PR is good

[davidebriani]

If the device is publishing the Stopped status when all resources are ready, we should update the deployment status to Stopped instead of doing nothing.

You consider using a cond statement here too:

with {:ok, deployment} <- Containers.fetch_deployment(deployment_id, tenant: tenant) do
  cond do
    status == "Starting" and deployment.status == :started ->
      # Skip Starting if already Started
      {:ok, deployment}

    status == "Stopping" and deployment.status == :stopped ->
      # Skip Stopping if already Stopped
      {:ok, deployment}
      
    status == "Error" ->
      # An error has occurred, set the deployment status immediately
      Containers.deployment_set_status(deployment, status, message, tenant: tenant)
      
    deployment.status in @initial_statuses ->
      # Besides errors, verify status of deployment resources before
      # blindly setting the deployment status. Indeed, a Stopped
      # status could be signaled before all resources are ready.
      Containers.deployment_update_status(deployment, tenant: tenant)
      
    true ->
      # Besides initial statuses, directly set the deployment status
      Containers.deployment_set_status(deployment, status, message, tenant: tenant)
  end
end

[noaccOS]

On deployment event we can only receive starting or stopping, not stopped
If all resources are ready and we receive a stopped on available_deployments, the update_status will correctly set the status to stopped and run the callbacks

[davidebriani]

Yes, I just realized the same thing.

We could still consider the cond statement if it makes the code clearer and less nested.

Regarding the Stopped status, it probably doesn't do any harm calling Containers.deployment_update_status(deployment, tenant: tenant) here too. It might serves us well if we get the device to also publish Stopped on DeploymentEvents as we were discussing the other day (because that would fix the fact that the Stopped status property on AvailableDeployments is not re-sent from Astarte and would let us avoid having the first two cond cases).

Just food for thought, the current code already works.