opensearch-project / terraform-provider-opensearch

https://registry.terraform.io/providers/opensearch-project/opensearch
Apache License 2.0
74 stars 57 forks source link

fix(opensearch-provider): fix working with opensearch 2.x.x #16

Closed serge-r closed 1 year ago

serge-r commented 1 year ago

Signed-off-by: serge-r zoneofmail@gmail.com

Description

Hello!

Some time ago, I tried to use this provider to support our Opensearch 2.4.0 infrastructure in Terraform. But I have found the next problems:

Perhaps there are more problems, but at this time I have tested only Indexes and IndexTemplates resources.

I know also about https://github.com/phillbaker/terraform-provider-elasticsearch, which this provider was forked from, but I think, that is more about Elasticsearch, not Opensearch. Unfortunately, this provider also has the same problems with version detection.

Please say what are you thinking about this PR. And also big thanks for your and Phillbaker's work!

Issues Resolved

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

haarchri commented 1 year ago

can we see this PR merged soon ?

prudhvigodithi commented 1 year ago

Hey thanks for the PR @serge-r and apologies for the long wait, ready to roll. @serge-r can you please fix the conflicts and we can merge this PR @phillbaker can you please review ? Thank you @bbarani @peterzhuamazon

serge-r commented 1 year ago

Thank you guys and sorry for delay, I will check and send a response ASAP

andrejvanderzee commented 1 year ago

For us, the resources opensearch_index_template and opensearch_dashboard_object are not working (applies successfully but no result in AWS OpenSearch 2.3):

resource "opensearch_index_template" "index_template" {
  for_each = var.indices
  name     = each.key
  body     = <<EOF
{
  "index_patterns": ["${each.key}*"],
  "template" : {
    "settings": {
      "plugins.index_state_management.rollover_alias": "${each.key}"
   }
  }
}
EOF
}

resource "opensearch_dashboard_object" "index_pattern" {
  for_each = var.indices
  body     = <<EOF
[
  {
    "_id": "index-pattern:${each.key}",
    "_type": "_doc",
    "_source": {
      "type": "index-pattern",
      "index-pattern": {
        "title": "${each.key}-*",
        "timeFieldName": "@timestamp"
      }
    }
  }
]
EOF
}

Happy to test when fixes are implemented.

justinhauer commented 1 year ago

@phillbaker @jackson-theisen @prudhvigodithi or @peterzhuamazon can one of you please review this? I also need this feature.

serge-r commented 1 year ago

Hello guys!

I have pushed fixed version. Please review.

@andrejvanderzee I tested opensearch_dashboard_object and opensearch_index_template with self-hosted opensearch and dashboard version 2.3, everything working normally. Unfortunately I have no ability to test it with AWS opensearch.

I think that need to use this version carefully, because Opensearch can use different API - at this time this provider using elastic7 library. Perhaps it will work better with native opensearch go lib.

andrejvanderzee commented 1 year ago

@serge-r The index templates are correctly created with your fork on AWS Opensearch 2.5. But, it keeps giving me changes after multiple applies:

  # opensearch_index_template.index_template["kube-shared"] will be updated in-place
  ~ resource "opensearch_index_template" "index_template" {
      ~ body = jsonencode(
          ~ {
              + index_patterns  = [
                  + "kube-shared*",
                ]
              - index_templates = [
                  - {
                      - index_template = {
                          - index_patterns = [
                              - "kube-shared*",
                            ]
                          - template       = {
                              - settings = {
                                  - index = {
                                      - number_of_replicas = "1"
                                      - number_of_shards   = "1"
                                      - plugins            = {
                                          - index_state_management = {
                                              - rollover_alias = "kube-shared"
                                            }
                                        }
                                      - refresh_interval   = "60s"
                                    }
                                }
                            }
                        }
                      - name           = "kube-shared"
                    },
                ] -> null
              + template        = {
                  + settings = {
                      + index                                           = {
                          + number_of_replicas = 1
                          + number_of_shards   = 1
                          + refresh_interval   = "60s"
                        }
                      + "plugins.index_state_management.rollover_alias" = "kube-shared"
                    }
                }
            }
        )
        id   = "kube-shared"
        name = "kube-shared"
    }
andrejvanderzee commented 1 year ago

@serge-r The problem is with keeping appplying non-changes while it already has been applied, on AWS you soon run into errors such as the below (for opensearch_index, which gives similar problem in the official beta-release):

Error: elastic: Error 429 (Too Many Requests): index [kubernetes-events-dev-000001] blocked by: [TOO_MANY_REQUESTS/12/disk usage exceeded flood-stage watermark, index has read-only-allow-delete block]; [type=cluster_block_exception]
│ 
│   with opensearch_index.bootstrap["kubernetes-events-dev"],
│   on indices.tf line 19, in resource "opensearch_index" "bootstrap":
│   19: resource "opensearch_index" "bootstrap" {

Would be nice if the above can be fixed.

andrejvanderzee commented 1 year ago

@phillbaker @prudhvigodithi @peterzhuamazon @jackson-theisen Can someone please review? Looks good to me at least until the Opensearch Go client is used by this provider.

prudhvigodithi commented 1 year ago

Hey @serge-r I see the tests are failing also, I assume the PR is not rebased can you please check? Also @phillbaker can you add your initial thoughts on this? Thank you @peterzhuamazon @bbarani

phillbaker commented 1 year ago

Thank you for the contribution. Right now, I'd like to focus on getting a v1 of this provider out the door. I'm happy to fully review this once that happens.

At a high level, this PR is pretty large and widely impacting. It should be broken up into separate PRs, one for each major change. E.g. a first PR for dropping elasticsearch 6 support, a second PR for deprecated API for index templates, etc. Those smaller PRs will also make merging individual fixes easier without having to wait for the entire PR to be ready.

Overall, these changes will need to have appropriate updates to the tests.

andrejvanderzee commented 1 year ago

@serge-r Do you have an example of opensearch_dashboard_object that works for you? This does not work for me:

resource "opensearch_dashboard_object" "index_pattern" {
  for_each = var.indices
  body     = <<EOF
[
  {
    "_id": "index-pattern:${each.key}",
    "_type": "_doc",
    "_source": {
      "type": "index-pattern",
      "index-pattern": {
        "title": "${each.key}-*",
        "timeFieldName": "@timestamp"
      }
    }
  }
]
EOF
}

It applies, but in AWS Opensearch 2.5 I see now saved object / index pattern.

serge-r commented 1 year ago

Hello!

Sorry for delay, unfortunately I can't spend many times on it(

Removing elastic6 it's already huge amount of changes, but I understand the problem and try to divide the PR to small PRs.

@andrejvanderzee about consistent changing errors - I will check, thank you for the info

about problem with opensearch_dashboard_object - check please which index in AWS using for store dashboard objects, perhaps it have different name. Default index in provider is ".dashboard". In AWS it can be ".kibana" or any other name. You can set dashboards index via index field in the resource like

resource "opensearch_dashboard_object" "index_pattern" {
  index = ".kibana"
  for_each = var.indices
  body     = <<EOF
[
  {
    "_id": "index-pattern:${each.key}",
    "_type": "_doc",
    "_source": {
      "type": "index-pattern",
      "index-pattern": {
        "title": "${each.key}-*",
        "timeFieldName": "@timestamp"
      }
    }
  }
]
EOF
}
andrejvanderzee commented 1 year ago

Thanks @serge-r adding index = ".kibana" made it work.

xsnrg commented 1 year ago

Any thoughts on when this might get merged and a new version made available?

prudhvigodithi commented 1 year ago

At a high level, this PR is pretty large and widely impacting. It should be broken up into separate PRs, one for each major change. E.g. a first PR for dropping elasticsearch 6 support, a second PR for deprecated API for index templates, etc. Those smaller PRs will also make merging individual fixes easier without having to wait for the entire PR to be ready.

Overall, these changes will need to have appropriate updates to the tests.

Hey @serge-r based on @phillbaker suggestion, can you please split the changes into different PR's and adding unit tests for each of them? I have created an issue to track the 2.x support https://github.com/opensearch-project/terraform-provider-opensearch/issues/39 please check. Also if you are ok we can close this PR and move forward with smaller PRs that can track 2.x support. Thank you @bbarani @peterzhuamazon @rishabh6788

serge-r commented 1 year ago

Hello guys, and sorry for waiting. I have made new PR with less amount of changes, please review.

https://github.com/opensearch-project/terraform-provider-opensearch/pull/41

bbarani commented 1 year ago

@prudhvigodithi @phillbaker Can we prioritize this PR? It would be beneficial for lots of users using 2.x version.

andrejvanderzee commented 1 year ago

+1

elchananmizrachi commented 1 year ago

@prudhvigodithi @phillbaker Can someone please take a look at this PR please?

markdboyd commented 1 year ago

What is the minimum change necessary to support Opensearch 2.x.x? Is it just this?:

version mismatch problem - Opensearch is using different version numbers, instead of Elasticsearch, but this provider is checking it only for Elastic, which makes this provider not usable for Opensearch.

Or do we need the Elastic 6/7 deprecations as well to support Opensearch 2.x.x?

justinhauer commented 1 year ago

totally appreciate everyone's time in the open source project, any updates on this?

shurkus commented 1 year ago

Any news?

stevesimpson418 commented 1 year ago

This would be huge for those in the community running OpenSearch 2.x.x

prudhvigodithi commented 1 year ago

Closing this PR, please check the latest PR that has all the support for 2.x https://github.com/opensearch-project/terraform-provider-opensearch/pull/62. Thank you

prudhvigodithi commented 1 year ago

Hey All, we just released the Beta version of OpenSearch Terraform Provider with 2.x support. We aim to release the non-beta version based on the feedback. Additionally, we encourage and welcome contributions from the community. If you come across any issues or wish to expand the coverage of APIs, we invite you to contribute to the project. Provider Download link: https://registry.terraform.io/providers/opensearch-project/opensearch/2.0.0-beta.1

Thank you