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

[Existing Resources] Add Team and Schedule #60

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

diraol
Copy link
Contributor

@diraol diraol commented Jun 14, 2024

what

Add the ability to "import" (data) existing scheduling and team.

why

This is utterly useful especially for migrations with resources managed manually or to integrate multiple teams that handle opsgenie differently (some automated, some manually).

references

@diraol diraol requested review from a team as code owners June 14, 2024 20:57
@diraol diraol requested review from Gowiem and jamengual June 14, 2024 20:57
@mergify mergify bot added the triage Needs triage label Jun 14, 2024
@diraol diraol force-pushed the dro/existing_resources branch 2 times, most recently from 01b604f to d062fda Compare June 17, 2024 19:46
@diraol
Copy link
Contributor Author

diraol commented Jul 23, 2024

This one should be "fixed" as soon as the other PRs are merged.

@diraol diraol force-pushed the dro/existing_resources branch 2 times, most recently from 49b8de5 to dcd536b Compare July 23, 2024 17:44
Copy link
Member

@Benbentwo Benbentwo left a comment

Choose a reason for hiding this comment

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

/terratest

@mergify mergify bot removed the triage Needs triage label Jul 23, 2024
@Benbentwo
Copy link
Member

/terratest

Copy link
Member

@Benbentwo Benbentwo left a comment

Choose a reason for hiding this comment

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

With the data source looked up we need a resource to import it to.

if these are for imports then these should be resources. they can include the import block.

though is there a reason the code in main branch cannot be used with a terraform import for the opsgenie schedule?

@mergify mergify bot added the triage Needs triage label Jul 23, 2024
@diraol
Copy link
Contributor Author

diraol commented Jul 23, 2024

With the data source looked up we need a resource to import it to.

if these are for imports then these should be resources. they can include the import block.

though is there a reason the code in main branch cannot be used with a terraform import for the opsgenie schedule?

Over here we are using a CI to run all opentofu procedures (plan/apply). Users do not have permission to run it locally, so import is not an options (building a pipeline to handle a 1 time import seems much more complex).

I just don't get why:

With the data source looked up we need a resource to import it to.

@Benbentwo
Copy link
Member

If we add the datasource for the opsgenie_schedule and opsgenie_team and then you run terraform import for them something like terraform import opsgenie_schedule.test <my schedule id>. 2 things will happen:

  1. Import will fail because you can only import into a resource not a datasource
  2. If you could import into a datasource, they are currently unused and would not affect anything.

The reason the existing_users.tf was there was because if there were already users, we look them up (not stored in state yet) and then use them instead for various resources. Essentially allowing other resources to be created and stored in state by directly creating a user, or looking them up.

Regarding your problem:

Over here we are using a CI to run all opentofu procedures (plan/apply). Users do not have permission to run it locally, so import is not an options (building a pipeline to handle a 1 time import seems much more complex).

I think you have 2 options.

  1. you need to terraform import with someone who has access to run locally - this is the cleanest way
  2. Destroy the resources by hand and recreate them using terraform - this is less ideal as you might lose configuration / have some down time.

@Benbentwo
Copy link
Member

The other option - is if we want to support lookups of teams and schedules, then existing_schedules = lookup(var.opsgenie_resources, "existing_schedules", []) should be pointing to the datasources and we also need to use that variable (existing_schedules)

Copy link

mergify bot commented Jul 25, 2024

💥 This pull request now has conflicts. Could you fix it @diraol? 🙏

@mergify mergify bot added the conflict This PR has conflicts label Jul 25, 2024
@diraol diraol force-pushed the dro/existing_resources branch from 5028880 to eb7251e Compare July 25, 2024 18:36
@mergify mergify bot removed the conflict This PR has conflicts label Jul 25, 2024
@diraol
Copy link
Contributor Author

diraol commented Jul 25, 2024

Conflicts fixed and I've also added some changes that were missing (again, forgot while splitting the PR) related to data objects.

See if it make sense @Benbentwo - If it doesn't, no hard feelings! :)

@diraol diraol requested a review from Benbentwo July 25, 2024 18:50
@Benbentwo
Copy link
Member

/terratest

Copy link
Member

@Benbentwo Benbentwo left a comment

Choose a reason for hiding this comment

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

This looks fine, tests are failing from conflicts (which isn't caused by this PR), config module is unfortunately one that we have tests disabled. we can roll this back if there's any issues or roll forward

@Benbentwo Benbentwo merged commit 671c38b into cloudposse:main Jul 26, 2024
69 of 71 checks passed
@mergify mergify bot removed the triage Needs triage label Jul 26, 2024
@diraol diraol deleted the dro/existing_resources branch July 29, 2024 14:47
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.

2 participants