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

fix(lateinit): skip version_id for firehose deliverystream #852

Merged

Conversation

haarchri
Copy link
Member

Description of your changes

When utilizing the official provider for the Firehose delivery stream, you can observe the CPU and memory consumption as follows:

kubectl top pods -n crossplane-system | grep "firehose"
official-provider-aws-firehose-4be7907cab90-65bf966f6f-g24kf   6414m    3150Mi

all Firehose delivery stream resources Synced and Ready true

dr9p7-v27jz True  True   arn:aws:firehose:eu-central-1:123456789101:deliverystream/eks-logging-to-splunk 
8hg7t-g2xl6 True  True   arn:aws:firehose:eu-central-1:123456789111:deliverystream/eks-logging-to-splunk 
fgjlt-8wblc True  True   arn:aws:firehose:eu-central-1:123456789121:deliverystream/eks-logging-to-splunk 
wbp2s-szw9g True  True   arn:aws:firehose:eu-central-1:123456789131:deliverystream/eks-logging-to-splunk 
fpdpz-jcvbf True  True   arn:aws:firehose:eu-central-1:123456789141:deliverystream/eks-logging-to-splunk 
m8v7f-ngxc8 True  True   arn:aws:firehose:eu-central-1:123456789151:deliverystream/eks-logging-to-splunk 
5zb25-89zgp True  True   arn:aws:firehose:eu-central-1:123456789161:deliverystream/eks-logging-to-splunk 
zxqqn-txlt5 True  True   arn:aws:firehose:eu-central-1:123456789181:deliverystream/eks-logging-to-splunk 
8j4jq-tvkvw True  True   arn:aws:firehose:eu-central-1:123456789191:deliverystream/eks-logging-to-splunk 
hhncg-zfd2t True  True   arn:aws:firehose:eu-central-1:123456789102:deliverystream/eks-logging-to-splunk 
hd7jl-kgfmp True  True   arn:aws:firehose:eu-central-1:123456789103:deliverystream/eks-logging-to-splunk 
vcrg5-xll4p True  True   arn:aws:firehose:eu-central-1:123456789104:deliverystream/eks-logging-to-splunk 

The observations under the given conditions are as follows:

 conditions:
 - lastTransitionTime: "2023-06-01T07:19:31Z"
  reason: ReconcileSuccess
  status: "True"
  type: Synced
 - lastTransitionTime: "2023-06-09T14:59:24Z"
  reason: Available
  status: "True"
  type: Ready
 - lastTransitionTime: "2023-08-29T13:25:15Z"
  reason: Ongoing
  status: "False"
  type: AsyncOperation
 - lastTransitionTime: "2023-07-24T12:11:54Z"
  message: 'apply failed: updating Kinesis Firehose Delivery Stream (eks-logging-to-splunk):
   ConcurrentModificationException: Cannot update firehose: eks-logging-to-splunk
   since the current version id: 2 and specified version id: 1 do not match: '
  reason: ApplyFailure
  status: "False"
  type: LastAsyncOperation

terraform plan shows the following:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:

 ~ update in-place
Terraform will perform the following actions:
# aws_kinesis_firehose_delivery_stream.hhncg-zfd2t will be updated in-place
 ~ resource "aws_kinesis_firehose_delivery_stream" "hhncg-zfd2t" {
    id       = "arn:aws:firehose:eu-central-1:123456789102:deliverystream/eks-logging-to-splunk"
    name      = "eks-logging-to-splunk"
    tags      = {
      "crossplane-kind"      = "[deliverystream.firehose.aws.upbound.io](http://deliverystream.firehose.aws.upbound.io/)"
      "crossplane-name"      = "hhncg-zfd2t"
      "crossplane-providerconfig" = "xxxx"
    }
   ~ version_id   = "2" -> "1"
    # (4 unchanged attributes hidden)
   ~ splunk_configuration {
     ~ hec_token         = "hec2-xxx" -> "hec6-yyy"
      # (5 unchanged attributes hidden)
     ~ cloudwatch_logging_options {
       ~ log_stream_name = "kinesis-error-stream" -> "destination-error-stream"
        # (2 unchanged attributes hidden)
      }
      # (1 unchanged block hidden)
    }
    # (2 unchanged blocks hidden)
  }
Plan: 0 to add, 1 to change, 0 to destroy.

after the fix for latinit we can see the following CPU and memory consumption

kubectl top pods -n crossplane-system | grep "firehose"
official-provider-aws-firehose-8ac2302d5278-5646d4bd5f-k4r59   187m      112Mi

Fixes #

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

NAME                                             READY   SYNCED   EXTERNAL-NAME                                                    AGE
deliverystream.firehose.aws.upbound.io/example   True    True     arn:aws:firehose:us-west-1:609897127049:deliverystream/example   65m

NAME                                  READY   SYNCED   EXTERNAL-NAME   AGE
role.iam.aws.upbound.io/sample-role   True    True     sample-role     65m

NAME                                READY   SYNCED   EXTERNAL-NAME   AGE
bucket.s3.aws.upbound.io/firehose   True    True     efirehose       65m
  Conditions:
    Last Transition Time:  2023-08-29T12:55:54Z
    Reason:                ReconcileSuccess
    Status:                True
    Type:                  Synced
    Last Transition Time:  2023-08-29T12:57:00Z
    Reason:                Available
    Status:                True
    Type:                  Ready
    Last Transition Time:  2023-08-29T12:56:23Z
    Reason:                Success
    Status:                True
    Type:                  LastAsyncOperation
    Last Transition Time:  2023-08-29T12:56:23Z
    Reason:                Finished
    Status:                True
    Type:                  AsyncOperation

Signed-off-by: Christopher Haar <christopher.haar@upbound.io>
@haarchri
Copy link
Member Author

/test-examples="examples/firehose/deliverystream.yaml"

Copy link
Collaborator

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ytsarev ytsarev merged commit 0f0d7a4 into crossplane-contrib:main Aug 30, 2023
10 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants