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

Add external metastores to azurerm_hdinsight_hadoop_cluster #6145

Merged
merged 12 commits into from
May 4, 2020

Conversation

kosinsky
Copy link
Contributor

@kosinsky kosinsky commented Mar 17, 2020

Added support for external metastores. Implements #5230
New optional block:

metastores {
    hive {
        server   = azurerm_sql_server.test.fully_qualified_domain_name
        database_name = azurerm_sql_database.test.name
        username      = azurerm_sql_server.test.administrator_login
        password      = azurerm_sql_server.test.administrator_login_password
    }
    oozie {
        server   = azurerm_sql_server.test.fully_qualified_domain_name
        database_name = azurerm_sql_database.test.name
        username      = azurerm_sql_server.test.administrator_login
        password      = azurerm_sql_server.test.administrator_login_password
    }
    ambari {
        server   = azurerm_sql_server.test.fully_qualified_domain_name
        database_name = azurerm_sql_database.test.name
        username      = azurerm_sql_server.test.administrator_login
        password      = azurerm_sql_server.test.administrator_login_password
   }
}

These metastores are optional and completely independent from each other

Fixes #5230

@kosinsky kosinsky changed the title Support for external Hive metastore [HDInsights] Support for external Hive metastore Mar 17, 2020
@kosinsky kosinsky force-pushed the hdi/external_metastore branch from 0c84ab5 to cbd1cde Compare March 17, 2020 19:43
@kosinsky kosinsky force-pushed the hdi/external_metastore branch from cbd1cde to b230942 Compare March 17, 2020 19:47
@ghost ghost added size/XL and removed size/L labels Mar 23, 2020
@kosinsky kosinsky changed the title [HDInsights] Support for external Hive metastore Add external metastores to azurerm_hdinsight_hadoop_cluster Mar 23, 2020
@kosinsky kosinsky force-pushed the hdi/external_metastore branch from 2437ccd to ce341b9 Compare March 23, 2020 21:20
AzureRMHDInsightHadoopCluster_requiresImport
@kosinsky
Copy link
Contributor Author

Ran (single threaded to avoid hitting core quota):

make testacc TEST='./azurerm/internal/services/hdinsight/tests/' TESTARGS='-run=TestAccAzureRMHDInsightHadoopCluster -parallel=1' TESTTIMEOUT='300m'

Results:

==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test ./azurerm/internal/services/hdinsight/tests/ -v -run=TestAccAzureRMHDInsightHadoopCluster -parallel=1 -timeout 300m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMHDInsightHadoopCluster_basic
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_basic
=== RUN   TestAccAzureRMHDInsightHadoopCluster_requiresImport
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_requiresImport
=== RUN   TestAccAzureRMHDInsightHadoopCluster_update
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_update
=== RUN   TestAccAzureRMHDInsightHadoopCluster_sshKeys
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_sshKeys
=== RUN   TestAccAzureRMHDInsightHadoopCluster_virtualNetwork
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_virtualNetwork
=== RUN   TestAccAzureRMHDInsightHadoopCluster_complete
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_complete
=== RUN   TestAccAzureRMHDInsightHadoopCluster_edgeNodeBasic
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_edgeNodeBasic
=== RUN   TestAccAzureRMHDInsightHadoopCluster_addEdgeNodeBasic
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_addEdgeNodeBasic
=== RUN   TestAccAzureRMHDInsightHadoopCluster_gen2storage
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_gen2storage
=== RUN   TestAccAzureRMHDInsightHadoopCluster_gen2AndBlobStorage
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_gen2AndBlobStorage
=== RUN   TestAccAzureRMHDInsightHadoopCluster_metastore
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_metastore
=== CONT  TestAccAzureRMHDInsightHadoopCluster_basic
--- PASS: TestAccAzureRMHDInsightHadoopCluster_basic (996.51s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_edgeNodeBasic
--- PASS: TestAccAzureRMHDInsightHadoopCluster_edgeNodeBasic (1458.67s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_metastore
--- PASS: TestAccAzureRMHDInsightHadoopCluster_metastore (1223.60s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_gen2AndBlobStorage
--- PASS: TestAccAzureRMHDInsightHadoopCluster_gen2AndBlobStorage (1358.88s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_gen2storage
--- PASS: TestAccAzureRMHDInsightHadoopCluster_gen2storage (1095.54s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_addEdgeNodeBasic
--- PASS: TestAccAzureRMHDInsightHadoopCluster_addEdgeNodeBasic (2749.66s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_sshKeys
--- PASS: TestAccAzureRMHDInsightHadoopCluster_sshKeys (995.11s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_complete
--- PASS: TestAccAzureRMHDInsightHadoopCluster_complete (3157.23s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_virtualNetwork
--- PASS: TestAccAzureRMHDInsightHadoopCluster_virtualNetwork (2491.03s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_update
--- PASS: TestAccAzureRMHDInsightHadoopCluster_update (1622.04s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_requiresImport
--- PASS: TestAccAzureRMHDInsightHadoopCluster_requiresImport (1047.37s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/hdinsight/tests	16972.04s

@ross-p-smith
Copy link
Contributor

Perfect - just what I am after for my project. Would love to see this

@kosinsky
Copy link
Contributor Author

kosinsky commented Apr 13, 2020

@katbyte @tombuildsstuff Could you take a look on this PR?

Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Hi @kosinsky
Thanks for this PR, it's off to a good start for a fairly tricky resource / API.

I've left a few comments in-line that need addressing to begin with. I have some concerns around update behaviours because of the nature of the API (and the absence of models for the configuration items covered here in the SDK). If you can address the comments I've left you'll be able to add tests for setting metastores individually (there should be one test that only sets one/two to prove the Optional behaviour), and at least one update test that modifies them I think that might shake out a few issues.

Thanks
Ste

azurerm/helpers/azure/hdinsight.go Show resolved Hide resolved
azurerm/helpers/azure/hdinsight.go Show resolved Hide resolved
azurerm/helpers/azure/hdinsight.go Show resolved Hide resolved
website/docs/r/hdinsight_hadoop_cluster.html.markdown Outdated Show resolved Hide resolved
kosinsky and others added 4 commits April 24, 2020 07:49
Co-Authored-By: Steve <11830746+jackofallops@users.noreply.github.com>
…m-provider-azurerm into hdi/external_metastore
@kosinsky
Copy link
Contributor Author

kosinsky commented Apr 25, 2020

make testacc TEST='./azurerm/internal/services/hdinsight/tests/' TESTARGS='-run=Metastore -parallel=1' TESTTIMEOUT='180m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test ./azurerm/internal/services/hdinsight/tests/ -v -run=Metastore -parallel=1 -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMHDInsightHadoopCluster_allMetastores
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_allMetastores
=== RUN   TestAccAzureRMHDInsightHadoopCluster_hiveMetastore
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_hiveMetastore
=== RUN   TestAccAzureRMHDInsightHadoopCluster_updateMetastore
=== PAUSE TestAccAzureRMHDInsightHadoopCluster_updateMetastore
=== CONT  TestAccAzureRMHDInsightHadoopCluster_allMetastores
--- PASS: TestAccAzureRMHDInsightHadoopCluster_allMetastores (1245.81s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_updateMetastore
--- PASS: TestAccAzureRMHDInsightHadoopCluster_updateMetastore (2193.06s)
=== CONT  TestAccAzureRMHDInsightHadoopCluster_hiveMetastore
--- PASS: TestAccAzureRMHDInsightHadoopCluster_hiveMetastore (990.66s)
PASS
ok    github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/hdinsight/tests     4430.51s

Added test for single metastore (Hive) as well as for updating. AFAIK, HDInsight doesn't allow to update metastore configuration for existing cluster as a result update will cause cluster replacement.

UI and doc has no mentions how to change metastore configuration: https://docs.microsoft.com/en-us/azure/hdinsight/hdinsight-use-external-metadata-stores

Setting metastore implemented via bootstrap settings https://docs.microsoft.com/en-us/azure/hdinsight/hdinsight-hadoop-customize-cluster-bootstrap Documentation doesn't mention any possibility to update these settings. I opened an issue to confirm: MicrosoftDocs/azure-docs#53255

Terraform resources for HDInsight usually require replacement due to API limitations except changes in tags, resizing (changing number of work nodes) and adding edge note.

cc @jackofallops

@kosinsky
Copy link
Contributor Author

@jackofallops I think I got confirmation that we need to replace cluster in case of metastore changes (MicrosoftDocs/azure-docs#53255 (comment)). This PR already supports it. Could you take one more look?

@jackofallops
Copy link
Member

Tests Pass:
image

@jackofallops jackofallops merged commit b16f7e5 into hashicorp:master May 4, 2020
@jackofallops jackofallops added this to the v2.9.0 milestone May 4, 2020
jackofallops added a commit that referenced this pull request May 4, 2020
@ghost
Copy link

ghost commented May 8, 2020

This has been released in version 2.9.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.9.0"
}
# ... other configuration ...

@rahulraj7-del
Copy link

@kosinsky Does this optional block for "metastore" holds true for "azurerm_hdinsight_spark_cluster" too. I see this is documented for Hadoop cluster but not for spark.

@kosinsky
Copy link
Contributor Author

Not yet. I'm working on PR to support it for other hdinsight cluster types

@ghost
Copy link

ghost commented Jun 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Jun 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for HDInsight Hadoop cluster external SQL database
5 participants