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

[Bug]: instanceRefresh is added back to AutoscalingGroup after removal #1631

Open
1 task done
r-nasiri opened this issue Jan 10, 2025 · 4 comments · May be fixed by #1645
Open
1 task done

[Bug]: instanceRefresh is added back to AutoscalingGroup after removal #1631

r-nasiri opened this issue Jan 10, 2025 · 4 comments · May be fixed by #1645
Labels
bug Something isn't working needs:triage

Comments

@r-nasiri
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Affected Resource(s)

  • autoscaling.aws.upbound.io/v1beta3 - AutoscalingGroup

Resource MRs required to reproduce the bug

apiVersion: autoscaling.aws.upbound.io/v1beta3
kind: AutoscalingGroup
metadata:
  name: test-asg
spec:
  forProvider:
    desiredCapacity: 0
    launchTemplate:
      id: lt-0ecc4c9aa799d7176
      version: "5"
    maxSize: 1
    minSize: 0
    instanceRefresh:
      preferences:
        instanceWarmup: "300"
        maxHealthyPercentage: 110
        minHealthyPercentage: 90
      strategy: Rolling
    region: us-west-2
    vpcZoneIdentifier:
    - subnet-04f72f198556cbb2c
  managementPolicies:
  - Create
  - Update
  - Delete
  - Observe
  - LateInitialize
  providerConfigRef:
    name: aws-provider-config

Steps to Reproduce

  • Create a AutoscalingGroup using the provided manifest. It should include instanceRefresh field.
  • Make sure the AutoscalingGroup is created and instanceRefresh shows up in status.atProvider
  • Remove instanceRefresh from the manifest and apply it again.
  • instanceRefresh is not removed and has previous values.

What happened?

When the LateInitialize is enabled, removing instanceRefresh doesn't update the ASG and gets added back because this is an async operation and spec.atProvider doesn't get updated right away and gets added back by LateInitialize process. I think this field should be added to LateInitialize IgnoreFields.

Relevant Error Output Snippet

No response

Crossplane Version

1.16.0

Provider Version

1.18.2

Kubernetes Version

1.29

Kubernetes Distribution

EKS

Additional Info

No response

@r-nasiri r-nasiri added bug Something isn't working needs:triage labels Jan 10, 2025
@r-nasiri r-nasiri linked a pull request Jan 18, 2025 that will close this issue
3 tasks
@turkenf
Copy link
Collaborator

turkenf commented Jan 29, 2025

Hi @r-nasiri,

Thank you for your issue report and first contribution in this PR. Could you please elaborate a bit more on the problem you are experiencing when the instanceRefresh field cannot be removed so that we can fully understand the issue?

@r-nasiri
Copy link
Author

hi @turkenf ,

The issue is that when you create an ASG and configure the instance refresh and later you decide to disable this feature, you need to remove instanceRefresh from your manifest. However, with the LateInitialize management policy enabled it doesn't allow you to do that because the change doesn't get updated in the status right away and LateInitialize adds it back from status.atProvider into spec.forProvider

@turkenf
Copy link
Collaborator

turkenf commented Jan 29, 2025

Thank you for your quick response @r-nasiri 🙌 I understand what you are saying, what I mean is, what is the disadvantage of not being able to remove/delete this field?

@r-nasiri
Copy link
Author

r-nasiri commented Jan 29, 2025

if you can't delete the field, you cannot disable auto refresh of ASG after it is enabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants