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

DXE-2430 Keep as much as possible traffic targets and liveness tests declarati… #404

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hightoxicity
Copy link

…on order

Try much as possible to preserve order of:

  • liveness tests in property
  • traffic targets in property
  • assignment in geomap

@hightoxicity
Copy link
Author

If we focus for example only on traffic targets: the way elements not found in state were appended was problematic since iteration was done over a map having datacenters ids as keys, and since a golang range operation over those items was forcing sorting by datacenter id asc.

@Slonimskaia Slonimskaia changed the title Keep as much as possible traffic targets and liveness tests declarati… DXE-2430 Keep as much as possible traffic targets and liveness tests declarati… Mar 14, 2023
@Slonimskaia
Copy link
Contributor

Hello @hightoxicity,

Thank you for your contribution. We will need some time to review these changes.

Best regards,
Tatiana

@lkowalsk-akamai-com
Copy link
Contributor

@hightoxicity , the config-gtm-api does not rely or expect a sort order of lists that it receives from clients. Hence my question, what issue you are seeing our trying to fix with this PR?

@hightoxicity
Copy link
Author

hightoxicity commented Mar 20, 2023

@hightoxicity , the config-gtm-api does not rely or expect a sort order of lists that it receives from clients. Hence my question, what issue you are seeing our trying to fix with this PR?

After having imported things living in akamai with terraform import, if I run a terraform plan, it triggers changes since input vars does not match the sorting forced in the terraform state file by the provider.

Behaviour expected here is the ability to produce terraform input vars with a sorting that matches the one that is done on akamai backend and that we see when we query directly the Akamai API:

egcurl --header 'Accept: application/vnd.config-gtm.v1.5+json' -X GET https://${AKAMAI_HOST}/config-gtm/v1/domains/${GTM_DOMAIN_NAME}

The issue I have without this PR is that things are sorting in the middle (into the state) with a logic that neither honor inputs ordering provided nor the one of entities stored at akamai side. So I got a bunch of modifications just having terraform import things and trying to get a neutral tf plan at first terraform run.

That provider (and more particularly the GTM code) does not allow us to get idempotency we expect when we do not start from scratch and have to deal with terraform import.

@dawiddzhafarov
Copy link
Contributor

Hello @hightoxicity,

In order to review the PR, we have to reproduce the issue first. Could You please share the configuration files and steps You took that resulted in the issue You are describing? Also, please provide the version of akamai terraform provider You are using. Any information about remote state attributes You are importing would be helpful (without sensitive information, for example number of geomap assignments)

Currently, there is a diff suppress functionality for traffic targets and geomap assignments order, but there is no such functionality for liveness tests. The order of those elements does not matter from the API perspective, hence changes in the local configuration have no impact on the result.

With regards,
Dawid

@hightoxicity
Copy link
Author

hightoxicity commented Jun 14, 2023

Reproduction steps consists in:

  • Create manually GTM properties on akamai side with several traffic targets (do not create it with your tf provider), do not sort targets by name
  • Import into your tf state those elements using terraform import
  • Then run terraform plan (you would expect to have nothing that is about to be changed but targets are sorted and it would like to force an update)

We expect no update after having imported things living in GTM

@mgwoj
Copy link
Contributor

mgwoj commented Jun 15, 2023

Thank you for providing implementation for the fix. May I ask to attach unit tests for those changes, please?

@joel-g
Copy link
Contributor

joel-g commented Aug 31, 2023

I have also observed the forced re-ordering of targets because of the use of an ordered data structure to store them in state when in fact GTM targets do not have an order.

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

Successfully merging this pull request may close these issues.

None yet

6 participants