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

Update resources state handling #2

Open
louich opened this issue Jun 10, 2019 · 5 comments
Open

Update resources state handling #2

louich opened this issue Jun 10, 2019 · 5 comments

Comments

@louich
Copy link
Contributor

louich commented Jun 10, 2019

Goals

Terraform plugins basics

  • The create function:
    • Should define the resource unique identifier with d.SetId("[ID]") to indicate the resource was successfully created.

      It uses SetId, a built-in function, to set the ID of the resource to the address. The existence of a non-blank ID is what tells Terraform that a resource was created. This ID can be any string value, but should be a value that can be used to read the resource again.

      ...

      Again, because of the call to SetId, Terraform believes the resource was created. When running plan, Terraform properly determines there are no changes to apply.

    • Should return the resource state function.
  • The destroy function:
    • Should destroy the resource.
    • Can call d.SetId("") to indicate that the resource was sucessfully deleted, but is not necessary since any non-error return value assumes the resource was deleted successfully.
    • Should never update any state on the resource.
  • The read function:
    • Should sync the local state with the actual state (upstream).
    • Should be a read-only operation and therefore Should never modify the real resource.
    • Should update the ID to blank, if the resource no longer exists (maybe it was destroyed out of band).

Proposed changes

Update resources create function to return the read function

As stated in the Terraform doc:

"The create and update function should always return the read function to ensure the state is reflected"
https://www.terraform.io/docs/extend/writing-custom-providers.html#defining-resources

Example from Terraform on table implementation:

func resourceServerCreate(d *schema.ResourceData, m interface{}) error {
        address := d.Get("address").(string)
        d.SetId(address)
        return resourceServerRead(d, m)
}

Suppress diffs for resource name case sensitivity

The resource name should be case insensitive as SQL language does not take capitalization into account and the KSQL server stores the value uppercased. No differences should therefore be seen between capitalization.

Update read function to alter the resource state

func resourceServerRead(d *schema.ResourceData, m interface{}) error {
  client := m.(*MyClient)

  // Attempt to read from an upstream API
  obj, ok := client.Get(d.Id())

  // If the resource does not exist, inform Terraform. We want to immediately
  // return here to prevent further processing.
  if !ok {
    d.SetId("")
    return nil
  }

  d.Set("address", obj.Address)
  return nil
}

All the schema fields should be updated from the upstream configuration for sync
purposes.

Update the state with the DESCRIBE infos

If the schema gets more complex it could be interesting to parse the DESCRIBE query content in order to resolve the resource infos.

KSQL DESCRIBE

Query:

{
  "ksql": "DESCRIBE [RESOURCE_NAME];",
  "streamsProperties": {}
}

Result:

[
    {
        "@type": "sourceDescription",
        "statementText": "DESCRIBE [RESOURCE_NAME];",
        "sourceDescription": {
            "name": "[RESOURCE_NAME]",
            "readQueries": [
              /* => resource dependencies */
              /* Other queries listening for data */
            ],
            "writeQueries": [
                {
                    "sinks": [
                        "[RESOURCE_NAME]"
                    ],
                    "id": "[QUERY_ID]",
                    "queryString": "CREATE ..." // Good state value to compare against for changes
                }
            ],
            "fields": [
                {
                    "name": "ROWTIME",
                    "schema": {
                        "type": "BIGINT",
                        "fields": null,
                        "memberSchema": null
                    }
                },
                {
                    "name": "ROWKEY",
                    "schema": {
                        "type": "STRING",
                        "fields": null,
                        "memberSchema": null
                    }
                },
                {
                    "name": "[CUSTOM_NAME]",
                    "schema": {
                        "type": "[CUSTOM_TYPES]",
                        "fields": "[CUSTOM_FIELDS]",
                        "memberSchema": "[CUSTOM_MEMBER_SCHEMA]"
                    }
                },
                ...
            ],
            "type": "[STREAM|TABLE]",
            "key": "[value from the `WITH KEY` option]",
            "timestamp": "",
            "statistics": "",
            "errorStats": "",
            "extended": false,
            "format": "[AVRO|JSON|DELIMITED]",
            "topic": "[THE_TOPIC_RELATED_TO_THE_OUTPUT_DATA]",
            "partitions": 0,
            "replication": 0
        }
    }
]

Resource state

Not all of the payload from the above is required and relevant regarding the the resource's state considering the actual schema and could be similar to:

{
  "name": sourceDescription.name,
  "query": sourceDescription.writeQueries[index_where_sinks_contains_res_name].queryString
}

It is to note that the query would only be present on a CREATE AS SELECT query type.

@louich
Copy link
Contributor Author

louich commented Jun 10, 2019

Hey, @Mongey this is some improvements I've started working on in the PR #3 and that I have time to spend on if you're interested.

Tell me what you think 😉

@louich
Copy link
Contributor Author

louich commented Jun 10, 2019

@Mongey is the KSQL cm-update-confluent still the branch for the terraformer plugin?

@Mongey
Copy link
Owner

Mongey commented Jun 10, 2019

@louich it should be on master now (but I will need to check quickly). Are you running into issues?

Haven't been able to read the full issue yet (sorry, busy day!)

@louich
Copy link
Contributor Author

louich commented Jun 10, 2019

It's not issues related to this one but I was only wondering what were your plans going forward knowing that the terraform plugin would beneficiate from DESCRIBE calls and it's not there for the moment in the ksql repo.

I have issues with the DROP for resources created with the CREATE AS SELECT that I think of opening an new issue for (directly into the ksql repo may be) because we need to call TERMINATE on the persistent queries before the DROP but I don't know where you would prefer these changes to be made.

I can open PRs for DESCRIBE and TERMINATE if you'd like to?

@louich
Copy link
Contributor Author

louich commented Jun 12, 2019

PR for DESCRIBE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants