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

DynamoDB GSI minor autoscaling update in AWS is interpreted by Pulumi as GSIs requiring major changes #2856

Closed
Tracked by #3872
chrishoward opened this issue Oct 6, 2023 · 8 comments
Assignees
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec resolution/duplicate This issue is a duplicate of another issue
Milestone

Comments

@chrishoward
Copy link

chrishoward commented Oct 6, 2023

What happened?

Current Behaviour

  • We are using Pulumi to deploy AWS DynamoDB databases. These databases have Global Secondary Indexes (GSI).
  • In our production environment we use AWS Application AutoScaling to auto scale our read/write capacities of the base table and the GSIs
  • Whenever the base table or GSI read/write capacity is autoscaled in AWS to accommodate for traffic, Pulumi state becomes out of sync as it is unaware of the changes. This is expected behaviour
  • However, each time a GSI read/write capacity is auto scaled in AWS, when you run a pulumi refresh and view the diff (before actually finishing the refresh), it shows the latest updated GSI moves to the top of the GSI array.
  • It seems Pulumi is using the order of this array to identify changes, instead of the name of the GSIs themselves.
  • This results in Pulumi suggesting it will make radical changes to the GSIs to get them into a synced state again, which is concerning for us as we are unsure what it will try and do with the GSIs in AWS
  • You can see in the following screenshot (a pulumi refresh diff) that in AWS the sectionIdIndex GSI readCapacity was updated from 50 to 54. As a result in Pulumi it shows as moving to the top of the GSI array.

Image

  • If I proceed with the pulumi refresh so that the Pulumi state matches AWS state (both have above readCapacity as 54). Then when I run a pulumi preview it suggests that the Pulumi program state that will get deployed will not only revert the readCapacity back to the base 50, but considers it an entirely new GSI due to the array order, and suggests deleting the old GSI and recreating a new one.

Image

Expected Behaviour
I would expect Pulumi to identify the GSIs using their index name or something like that, and therefore the refresh diff or the preview would suggest that only the read/write capacities will be out of sync or require changing, not the entire GSIs

Example

Our DynamoDB Table Resource

const courseTable = new aws.dynamodb.Table(
  `${projectConfig.courseName}`,
  {
    name: `${projectConfig.courseName}`,
    attributes: [
      {
        name: 'id',
        type: 'S',
      },
      {
        name: 'tenantId',
        type: 'S',
      },
      {
        name: 'courseId',
        type: 'S',
      },
      {
        name: 'sectionId',
        type: 'S',
      },
      {
        name: 'slug',
        type: 'S',
      },
    ],
    billingMode: projectConfig.isProd ? 'PROVISIONED' : 'PAY_PER_REQUEST',
    globalSecondaryIndexes: [
      {
        hashKey: 'courseId',
        name: indexNames.licensingCourseServiceCourseIdIndex,
        projectionType: 'ALL',
        rangeKey: 'id',
        readCapacity: projectConfig.isProd ? 10 : undefined,
        writeCapacity: projectConfig.isProd ? 10 : undefined,
      },
      {
        hashKey: 'tenantId',
        name: indexNames.licensingCourseTenantIdIndex,
        projectionType: 'ALL',
        readCapacity: projectConfig.isProd ? 10 : undefined,
        writeCapacity: projectConfig.isProd ? 10 : undefined,
      },
      {
        hashKey: 'sectionId',
        name: indexNames.licensingCourseServiceSectionIdIndex,
        projectionType: 'ALL',
        rangeKey: 'id',
        readCapacity: projectConfig.isProd ? 50 : undefined,
        writeCapacity: projectConfig.isProd ? 10 : undefined,
      },
      {
        hashKey: 'slug',
        name: indexNames.licensingCourseServiceSlugIndex,
        projectionType: 'ALL',
        rangeKey: 'id',
        readCapacity: projectConfig.isProd ? 10 : undefined,
        writeCapacity: projectConfig.isProd ? 10 : undefined,
      },
    ],
    hashKey: 'id',
    readCapacity: projectConfig.isProd ? 50 : undefined,
    writeCapacity: projectConfig.isProd ? 10 : undefined,
    pointInTimeRecovery: projectConfig.isProd ? { enabled: true } : undefined,
    deletionProtectionEnabled: projectConfig.isProd,
  },
  commonTableOptions
  );

Our Application AutoScaling Resource Creator Function

const createDynamodbIndexAutoScaler = (
  tableName: string,
  indexName: string,
  readOrWrite: 'Read' | 'Write',
  minScale: number,
  maxScale: number,
  target: number = 70
) => {
  const dynamodbTableTarget = new aws.appautoscaling.Target(
    `${tableName}-dynamodbTable${readOrWrite}Target-index-${indexName}`,
    {
      maxCapacity: maxScale,
      minCapacity: minScale,
      resourceId: `table/${tableName}/index/${indexName}`,
      scalableDimension: `dynamodb:index:${readOrWrite}CapacityUnits`,
      serviceNamespace: 'dynamodb',
    }
  );
  const dynamodbTablePolicy = new aws.appautoscaling.Policy(
    `${tableName}-dynamodbTable${readOrWrite}TargetPolicy-index-${indexName}`,
    {
      policyType: 'TargetTrackingScaling',
      resourceId: dynamodbTableTarget.resourceId,
      scalableDimension: dynamodbTableTarget.scalableDimension,
      serviceNamespace: dynamodbTableTarget.serviceNamespace,
      targetTrackingScalingPolicyConfiguration: {
        predefinedMetricSpecification: {
          predefinedMetricType: `DynamoDB${readOrWrite}CapacityUtilization`,
        },
        targetValue: target,
      },
    }
  );
};

Our Application AutoScaling Resource Creator Function being Invoked

createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseServiceCourseIdIndex, 'Read', 10, 1000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseServiceCourseIdIndex, 'Write', 10, 1000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseServiceSectionIdIndex, 'Read', 50, 5000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseServiceSectionIdIndex, 'Write', 10, 1000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseServiceSlugIndex, 'Read', 10, 1000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseServiceSlugIndex, 'Write', 10, 1000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseTenantIdIndex, 'Read', 10, 1000);
createDynamodbIndexAutoScaler(courseName, indexNames.licensingCourseTenantIdIndex, 'Write', 10, 1000);

Output of pulumi about

CLI
Version      3.37.1
Go Version   go1.17.12
Go Compiler  gc

Plugins
NAME    VERSION
aws     5.42.0
awsx    1.0.2
docker  3.6.1
nodejs  unknown

Host
OS       ubuntu
Version  22.04
Arch     x86_64

This project is written in nodejs: executable='/home/[redacted]/.nvm/versions/node/v16.20.1/bin/node' version='v16.20.1'

Backend
Name           pulumi.com
URL            https://app.pulumi.com/[redacted] (Please advise if you need this value)
User           [redacted] (Please advise if you need this value)
Organizations  [redacted] (Please advise if you need this value)

Dependencies:
NAME                      VERSION
@pulumi/pulumi            3.24.1
@types/mime               2.0.3
@types/node               14.18.12
@types/recursive-readdir  2.2.0
aws-sdk                   2.1424.0
recursive-readdir         2.2.2
@pulumi/aws               5.42.0
@pulumi/awsx              1.0.2

Pulumi locates its logs in /tmp by default
warning: Failed to get information about the current stack: No current stack
warning: A new version of Pulumi is available. To upgrade from version '3.37.1' to '3.86.0', run
   $ curl -sSL https://get.pulumi.com | sh
or visit https://pulumi.com/docs/reference/install/ for manual instructions and release notes.

Additional context

Other Similar Issues
Another user mentions this as 'Issue #2' in their Github issue: pulumi/pulumi-aws-native#989

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@chrishoward chrishoward added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 6, 2023
@mikhailshilkov
Copy link
Member

@t0yv0 as our expert in all things diffs - do you feel it's a Pulumi-side issue? Any related issues in the bridge? TF provider does seem to mark it as TypeSet, so I'd naively expect the diff to be set-based https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/dynamodb/table.go#L158.

@t0yv0
Copy link
Member

t0yv0 commented Oct 6, 2023

I'll take a closer look. Started pulumi/pulumi-terraform-bridge#1417 tracking issue, I've seen at least one more issue with sets affecting diff results, could be related.

@t0yv0 t0yv0 modified the milestones: 0.95, 0.96 Oct 6, 2023
@chrishoward
Copy link
Author

Thanks @mikhailshilkov @t0yv0
Let me know if you need anything further.

@mikhailshilkov
Copy link
Member

Thank you @chrishoward, I think we are good for now. Adding this to our planning backlog.

@mikhailshilkov mikhailshilkov added bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. and removed needs-triage Needs attention from the triage team labels Oct 9, 2023
@mikhailshilkov mikhailshilkov removed this from the 0.96 milestone Oct 23, 2023
@chrishoward
Copy link
Author

@mikhailshilkov Curious what the progress on this is please, just so I can prioritise work accordingly internally in my team (ie. wait for the fix if it's coming soon, or find a workaround if it's a long way off). Thanks

@chrishoward
Copy link
Author

@mikhailshilkov @t0yv0 Curious if there has been progress on this please? All good if not, just want to know so our team can decide how to move forward with the issue. Thanks in advance

@t0yv0
Copy link
Member

t0yv0 commented Mar 12, 2024

Hi @chrishoward I have been going through set handling in bridged providers recently and I think I have some suggestions here, although I have not yet delved deeper into reproducing the issue with your particular setups.

There are two levels to the issue, semantic level and presentation level. It appears that current Pulumi behavior is reasonable as to what it is actually doing, and it will be difficult to change that, however we do have an opportunity to make the presentation more intuitive.

IN terms of moving forward for your team/workaround - the Semantics section is most relevant.

Semantics

It appears that autoscaling and refresh make your state inconsistent with your program, like this:

  sequenceDiagram
      participant Autoscaling
      participant Cloud
      participant State
      participant Program
      Autoscaling->>Cloud: readCapacity := 54
      Cloud->>State: pulumi refresh sets readCapacity := 54
      Program->>State: pulumi up sets readCapacity := 50
Loading

To accept the readCapacity value of 54 you need to edit the Pulumi program as there is no command that does that. There is pulumi import that can suggest program edits but a human needs to accept them into the source.

Perhaps ignoreChanges can be a useful workaround to suppress visible diffs in this case while still doing pulumi refresh.

Presentation

The diffs appear to be reordering elements in a list. We could possibly do better here, though it is not an easy fix. TF has a native concept of sets with customizable identity while Pulumi does not. As a result, Pulumi represents TF sets as lists. Both TF and bridged Pulumi providers canonicalize the set element ordering so that simply reordering set elements or adding more than one identical element becomes a no-op change. Unfortunately the diff presentation is assuming these are lists and appears to show reordering them.

@t0yv0
Copy link
Member

t0yv0 commented Mar 12, 2024

I've filed pulumi/pulumi-terraform-bridge#1755 as an interesting usability improvement we could undertake in the bridge.

@t0yv0 t0yv0 added the resolution/duplicate This issue is a duplicate of another issue label Mar 29, 2024
@t0yv0 t0yv0 mentioned this issue Apr 26, 2024
5 tasks
@t0yv0 t0yv0 closed this as completed May 7, 2024
@t0yv0 t0yv0 self-assigned this May 7, 2024
@t0yv0 t0yv0 added this to the 0.104 milestone May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/diff kind/bug related to Pulumi generating wrong diffs on preview or up. kind/bug Some behavior is incorrect or out of spec resolution/duplicate This issue is a duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants