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

Status change in account table fails for leased accounts that are expired #344

Closed
dsg9321 opened this issue Apr 21, 2020 · 11 comments · Fixed by #353
Closed

Status change in account table fails for leased accounts that are expired #344

dsg9321 opened this issue Apr 21, 2020 · 11 comments · Fixed by #353
Labels
question Further information is requested

Comments

@dsg9321
Copy link

dsg9321 commented Apr 21, 2020

Version information

  • DCE-CLI version : 0.4.0.
    -DCE- Backend version : 0.29.0
  • Go version : 1.13.10
  • Terraform version : 0.12.19

Deployed DCE successfully using terraform. Making use of DCE API's to perform operations to add accounts and to lease. Below are the points that explains the issue.

  1. Create lease API leases different account which is not specified in the json body.Account ID that was leased was different from that of the account id in request.
  2. Status of leased accounts in accounts table is not updated to ready status after the account got expired.The status is updated to 'Expired' in leases table but not in the account table which still remains as 'leased' status.

We wanted to make use of accounts that are created out of Control Tower to be added into the account pools of DCE.Please let us know if this is supported. Also interested in knowing the release date of DCE backend version 0.30.0

Please help me out in fixing this issues.

@eschwartz eschwartz added the question Further information is requested label Apr 22, 2020
@eschwartz
Copy link
Contributor

Hi @dsg9321, I'll try to answer these questions for you:

  1. Create lease API leases different account which is not specified in the json body.Account ID that was leased was different from that of the account id in request.

It sounds like you're trying to request a lease against a specific AWS account id. DCE does not currently support this. Instead, when you request a lease, it finds the first Ready account in the account pool, and assigns that account to your lease.

  1. Status of leased accounts in accounts table is not updated to ready status after the account got expired.The status is updated to 'Expired' in leases table but not in the account table which still remains as 'leased' status.

Interesting, this sounds like it could be a bug, but I think I need to better understand the situation.
Can you please walk through the reproduction steps (API requests or CLI commands) here in more detail, and describe/copy+paste exactly what you're seeing?

We wanted to make use of accounts that are created out of Control Tower to be added into the account pools of DCE.Please let us know if this is supported.

I'm not super familiar with Control Tower, so I can't answer this definitively. I would be interested to hear if you're running into any issues with this.
I will say that if you have resources in your accounts that you don't want to be destroyed at the end of every lease, you should filter these out in your aws-nuke configuration.

See https://dce.readthedocs.io/en/latest/howto.html#account-resets

Also interested in knowing the release date of DCE backend version 0.30.0

I can't give you a firm date, but it should be in the next week or two.

@eschwartz
Copy link
Contributor

Also interested in knowing the release date of DCE backend version 0.30.0

cc @AmudaPalani ?

@rafabnunes
Copy link

Hey @eschwartz

The scenario bellow reported previously by @dsg9321 it's also happening in my dce environment.

"Status of leased accounts in accounts table is not updated to ready status after the account got expired.The status is updated to 'Expired' in leases table but not in the account table which still remains as 'leased' status."

I want to include another scenario when an account exceeded the budget, the account only stay with status overbudget. The account does not transition to destroyed and hence does not back to the pool.

@dsg9321
Copy link
Author

dsg9321 commented Apr 23, 2020

Hi @eschwartz,

Thanks for the response.
I am reproducing the steps that I followed to add an account to account pool and lease it.

Below are the use cases for which I did testing on the accounts that are leased.

  1. Check for expiration of leased accounts.
  2. Check for 'Overbudget' of leased accounts.
  3. Destroy leased account manually before any of above use cases are satisfied.

Results.

  1. Account Expiry : As explained in previous comment, the status is not changed in accounts table.
  2. Overbudget : Similar issue as reported by @rafabnunes . The status in lease table changed to 'Inactive' and 'Overbudget', but the status in accounts table remains as ‘leased’.Also there was no notification sent to the mail id.
  3. Removing leased accounts manually works as expected.

Below are the steps followed.

  1. Added account to the pool.
  • API : https://[hostapi_url]/api/accounts
  • Body :
    {
    "adminRoleArn": "arn:aws:iam::29xxxxxxxx06:role/[admin_role from child account]",
    "id": "29xxxxxxxx06"
    }
  • Response :
    {
    "id": "29xxxxxxxx06",
    "accountStatus": "NotReady",
    "lastModifiedOn": 1587469592,
    "createdOn": 1587469592,
    "adminRoleArn": "arn:aws:iam::29xxxxxxxx06:role/[admin_role from child account]",
    "principalRoleArn": "arn:aws:iam::29xxxxxxxx06:role/DCEPrincipal-dcetestv2",
    "principalPolicyHash": ""507f91cb60e2c287926ceefbf059d509""
    }
  1. Leased the account.
  • API: https://[hostapi_url]/api/leases.
  • Picks one of the accounts from account pool with ‘Ready Status’.
  • Response:
    {
        "accountId": "29xxxxxxxx06",
        "principalId": "DCEPrincipal-dceSandbox-8",
        "id": "d022736f-de74-4e8c-bd26-2439533e18ca",
        "leaseStatus": "Active",
        "leaseStatusReason": "Active",
        "createdOn": 1587469930,
        "lastModifiedOn": 1587469930,
        "budgetAmount": 2,
        "budgetCurrency": "USD",
        "budgetNotificationEmails": [
            “[email_id]”
        ],
        "leaseStatusModifiedOn": 1587469930,
        "expiresOn": 1587780004
    }

Please take a look at the screenshot of Dynamodb table attached in the comment for detailed information.
Dynamodb

@eschwartz
Copy link
Contributor

@dsg9321 , @rafabnunes thanks for all the details. I think I'm understanding this now then:
after your lease is expired, the lease record gets marked as Inactive, but the account record is still marked as Leased.

This is definitely not expected behavior. The account should be marked as NotReady, and the Ready once reset is complete.

I can look into this a bit.

In the meantime, it would be good if you can check and see if there are any errors in your logs. If you have the tfvar cloudwatch_dashboard_toggle=true, any errors should show up in a CloudWatch dashboard. Otherwise, you'll have to write your own cloudwatch insights queries. The first place I'd look would be the update_lease_status lambda.

@dsg9321
Copy link
Author

dsg9321 commented Apr 24, 2020

@eschwartz Attaching the screenshot from cloudwatch dashboard.An error found in 'fan_out_update_lease_status' lambda.
error

@rafabnunes
Copy link

rafabnunes commented Apr 24, 2020

Hi @dsg9321 @eschwartz
The lambda is empty. I fixed this issue uploading the code for each lambda from the following path .dce/dce/scripts/artifacts/lambda.

My case lambda is working, but when an account change status to "OverBudget", does not proceed to destroyed and then back to the pool. In the screenshot it's an example about that.
In this scenario the account is inactive, the user will not able to login, but the resources still there and charging by AWS.

Screen Shot 2020-04-24 at 10 18 18

Screen Shot 2020-04-24 at 12 39 52

Screen Shot 2020-04-24 at 12 49 13

Screen Shot 2020-04-24 at 12 50 49

@eschwartz
Copy link
Contributor

@dsg9321 I want to let you know that I'm moving off the DCE team at Optum this week, so I want to pass this PR off @robologic to shepherd through.

@robologic can you take the ball on this one, please? I'm hoping this will all be resolved by #302 , but it's worth a follow-up

@dsg9321
Copy link
Author

dsg9321 commented Apr 30, 2020

@eschwartz thanks for notifying. @robologic looking forward to receiving your update on resolving the issue.

@dsg9321
Copy link
Author

dsg9321 commented May 19, 2020

Hi @robologic. Any progress on this issue?

@AmudaPalani AmudaPalani linked a pull request May 19, 2020 that will close this issue
9 tasks
AmudaPalani added a commit to AmudaPalani/dce that referenced this issue May 19, 2020
@AmudaPalani AmudaPalani linked a pull request May 20, 2020 that will close this issue
9 tasks
AmudaPalani added a commit to AmudaPalani/dce that referenced this issue May 29, 2020
@AmudaPalani AmudaPalani linked a pull request May 29, 2020 that will close this issue
9 tasks
AmudaPalani added a commit that referenced this issue Jun 1, 2020
* fixed update to account status

* updated changelog

* Updated functional test with bug fix for #344
@rafabnunes
Copy link

Hi @AmudaPalani

This update will fix the issue when leased account occurs overbudget ?
I've been asking because I've installed the version 0.30.1 and when leased account occurs overbudget, the account does not wiped.

As workaround, I've installed the version from link bellow. However the budget notification isn't working.
https://github.com/Optum/dce/tree/Feature/usage2.0

Thanks in advance for your reply.

Rafael Nunes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
3 participants