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] Terraform wants to add new Fabric Enviroments that already exists if Capacity is Paused #141

Open
1 task done
SemUijen opened this issue Dec 11, 2024 · 6 comments
Labels
bug Something isn't working waiting-for-author

Comments

@SemUijen
Copy link

SemUijen commented Dec 11, 2024

🐛 What happened?

When the Fabric Capacity is paused, the terraform provider wants to add already existing items. Currently, I only have Fabric Environments deployed so this issue might also occur for other Fabric Items(Lakehouses, notebooks, etc.).

If the Capacity is paused, the api will return 404 which might lead to the behavior of terraform wanting to deploy the item again
If this is the case, the terraform provider might need to check first if the Capacity is "running" before planning changes

🔬 How to reproduce?

  1. deploy Fabric Environment using terraform provider
  2. Pauze the Fabric Capacity
  3. Run terraform plan/apply again

🏗️ Code Sample / Log

No response

📷 Screenshots

No response

📈 Expected behavior

data "fabric_capacity" "example" {
  display_name = var.capacity_name

  lifecycle {
    postcondition {
      condition = self.state == "Active"
      error_message = "Capacity is not Active"
    }
  }
}if data_source 

I added the following to create the expected behavior. This fixes the problem but it might be better to have the terraform provider to do this instead of a postcondition

🌌 Environment (Provider Version)

0.1.0-beta.5

🌌 Environment (Terraform Version)

1.10

🌌 Environment (OS)

macOS

📎 Additional context

No response

🔰 Code of Conduct

  • I agree to follow this project's Code of Conduct.
@SemUijen SemUijen added the bug Something isn't working label Dec 11, 2024
@DariuszPorowski
Copy link
Member

Hi @SemUijen

I appreciate your idea, but it seems more like a feature request than a bug. There are some potential issues:

  • The built-in "keeper" only works with data "fabric_capacity".
    • It won't apply if someone provides the capacity ID directly.
  • Sometimes data "fabric_capacity" is used to get other properties, such as capacity state as you mentioned just to retrieve the actual regardless of value.
    • If we implement this feature and always fail when the capacity is not active, it will disrupt the data source's purpose.

I think lifecycle with postcondition as you proposed is a good reference how to handle it as own gate-keeper. I proposed this a part of our quick-start example: microsoft/fabric-terraform-quickstart#37

@SemUijen
Copy link
Author

Hi @DariuszPorowski

Those are indeed fair points! I agree that always failing if the capacity is not active is not the correct way to move forward.
My Expected behavior that I mention above, was more meant as an example of how I solved it for now.
It wil indeed not solve the two potential issues. I think the reference in the quickstart is a good addition but also doesn't fully solve the problem.

For me the expected behavior, should not be integrated in the data fabric_capacity but in the resource fabric_resource
As the behavior of provider wanting to add/deploy existing resources, because the API returns 404 as a result of the capacity not being active seems strange. Therefore, a Check, Warning or block while Reading(getting) resource information that checks if the capacity is active seems better to me.
I believe this will remove the confusion and will not disrupt any wanted behavior.

what do you think?

@DariuszPorowski
Copy link
Member

hi @SemUijen
Just thinking about the design of this feature, and not sure about solution. To make it happen each fabric_resource has to include Capcacity ID as mandatory, i.e. currently env required 2 attributes:

resource "fabric_environment" "example" {
  display_name = "example"
  workspace_id = "00000000-0000-0000-0000-000000000000"
}

after your solution it will add one more for capacity:

resource "fabric_environment" "example" {
  display_name = "example"
  workspace_id = "00000000-0000-0000-0000-000000000000"
  capacity_id = "00000000-0000-0000-0000-000000000000"
}

and under the hood each time the call to the API for capacity check will be executed, so basically we will double of the requests per each resource execution and that will cause throttling very easily.

@SemUijen
Copy link
Author

Hi @DariuszPorowski

Good point. This would lead too many unnecessary calls. I was thinking about using it in the resource "fabric_workspace" as the capacity id is mandatory for a workspace.
On the other hand, the 'resource "fabric_workspace"' or 'data "fabric_workspace"' is on itself not necessary.
As the workspace id can also directly be added to resources

resource "fabric_environment" "example" {
  display_name = "example"
  workspace_id = "00000000-0000-0000-0000-000000000000"
}

Maybe it should be caught in the error handling, but I do not think that the Rest APIs returns a specific error code/message for an inactive capacity

@DariuszPorowski
Copy link
Member

@SemUijen actually workspace does not require capacity id and can be created without it.

@SemUijen
Copy link
Author

Hi @DariuszPorowski
That is true, but in that scenario, the workspace would not be deployed on the fabric capacity, right? So, the fabric capacity being on pause would be an issue

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

No branches or pull requests

2 participants