search

openshift / aws-account-operator

Operator to manage pool of AWS accounts for Hive
Apache License 2.0
32 stars 75 forks source link

Update create account script to stop nuking existing account #633

Closed dkeohane closed 3 years ago

dkeohane commented 3 years ago

Encountered an issue today when calling make create-accountclaim when an account CR with a name of osd-creds-mgmt-osd-staging-1 already exists, the call nukes the existing account and replaces it with a templated account CR.

Fixed by adding a check to see if account already exists before applying new account CR.

iamkirkbater commented 3 years ago

I'm confused what the actual problem is here. The oc apply should be idempotent and should update the existing CR if it already exists or create a new one if it doesn't. What was the error you were seeing?

dkeohane commented 3 years ago

The issue I was encountering was when I had the existing Account CR osd-creds-mgmt-osd-staging-1 with a valid iamUserSecret set. The oc apply appears to wipe the iamUserSecret and therefore when the new accountclaim see there's a 'valid' account, it claims fine but fails when attempting createIAMSecret in the accountclaim_controller.go

iamkirkbater commented 3 years ago

Hmm, that sounds concerning. Is the iamUserSecret part of the Spec? If so, that would explain why that would happen given the oc process | oc apply and it's not a bug. If that's not part of the spec, then this needs to be filed as a bug and we need to investigate this further.

If you can confirm that this is part of the spec that's getting overwritten, go ahead and remove the hold and we can let this merge. Otherwise, please close this PR and open a new JIRA with the bug description.

/hold /lgtm

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dastergon, dkeohane, iamkirkbater

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/aws-account-operator/blob/master/OWNERS)~~ [iamkirkbater] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
dkeohane commented 3 years ago

Confirmed it's part of spec, removing hold