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

Enhances index, get, and delete snapshot management apis #369

Merged
merged 5 commits into from
May 24, 2022

Conversation

downsrob
Copy link
Contributor

@downsrob downsrob commented May 19, 2022

Signed-off-by: Clay Downs [email protected]

Issue #, if available:
#280

Description of changes:
Fills out the index, get, and delete apis and adds in unit and integration tests.

Index POST _plugins/_sm/policies/test-policy
{
    "description": "Daily snapshot policy",
    "creation": {
      "schedule": {
        "cron": {
          "expression": "0 8 * * *",
          "timezone": "UTC"
        }
      }
    },
    "deletion": {
      "condition": {
        "max_age": "7d",
        "max_count": 21,
        "min_count": 7
      }
    },
    "snapshot_config": {
        "date_math": "{now/d}",
        "indices": "*",
        "repository": "s3-repo",
        "ignore_unavailable": "true",
        "partial": "true",
        "metadata": {
            "taken_by": "sm"
        }
    }
}
...
{
  "_id" : "test-policy-sm-policy",
  "_version" : 1,
  "_seq_no" : 0,
  "_primary_term" : 1,
  "sm" : {
    "name" : "test-policy",
    "description" : "Daily snapshot policy",
    "creation" : {
      "schedule" : {
        "cron" : {
          "expression" : "0 8 * * *",
          "timezone" : "UTC"
        }
      }
    },
    "deletion" : {
      "schedule" : {
        "cron" : {
          "expression" : "0 1 * * *",
          "timezone" : "UTC"
        }
      },
      "condition" : {
        "max_count" : 21,
        "max_age" : "7d",
        "min_count" : 7
      }
    },
    "snapshot_config" : {
      "indices" : "*",
      "metadata" : {
        "taken_by" : "sm"
      },
      "ignore_unavailable" : "true",
      "repository" : "s3-repo",
      "partial" : "true",
      "date_math" : "{now/d}"
    },
    "schedule" : {
      "interval" : {
        "start_time" : 1652979214228,
        "period" : 1,
        "unit" : "Minutes"
      }
    },
    "enabled" : true,
    "last_updated_time" : 1652979214228,
    "enabled_time" : 1652979214227
  }
}

In this pull request, get only may be used with a single policy. It will be expanded to include wildcards and lists of policies in a future PR.

Get GET _plugins/_sm/policies/test-policy
...
{
  "_id" : "test-policy-sm-policy",
  "_version" : 1,
  "_seq_no" : 0,
  "_primary_term" : 1,
  "sm" : {
    "name" : "test-policy",
    "description" : "Daily snapshot policy",
    "creation" : {
      "schedule" : {
        "cron" : {
          "expression" : "0 8 * * *",
          "timezone" : "UTC"
        }
      }
    },
    "deletion" : {
      "schedule" : {
        "cron" : {
          "expression" : "0 1 * * *",
          "timezone" : "UTC"
        }
      },
      "condition" : {
        "max_count" : 21,
        "max_age" : "7d",
        "min_count" : 7
      }
    },
    "snapshot_config" : {
      "indices" : "*",
      "metadata" : {
        "taken_by" : "sm"
      },
      "ignore_unavailable" : "true",
      "repository" : "s3-repo",
      "partial" : "true",
      "date_math" : "{now/d}"
    },
    "schedule" : {
      "interval" : {
        "start_time" : 1652979214228,
        "period" : 1,
        "unit" : "Minutes"
      }
    },
    "enabled" : true,
    "last_updated_time" : 1652979214228,
    "enabled_time" : 1652979214227
  }
}
Delete DELETE _plugins/_sm/policies/test-policy
...
{
  "_index" : ".opendistro-ism-config",
  "_id" : "test-policy-sm-policy",
  "_version" : 2,
  "result" : "deleted",
  "forced_refresh" : true,
  "_shards" : {
    "total" : 2,
    "successful" : 1,
    "failed" : 0
  },
  "_seq_no" : 7,
  "_primary_term" : 1
}
*CheckList:* - [X] Commits are signed per the DCO using --signoff

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.

@downsrob downsrob marked this pull request as ready for review May 19, 2022 17:29
@downsrob downsrob requested review from a team, bowenlan-amzn and dbbaughe May 19, 2022 17:29
@downsrob downsrob changed the title Enhances index, get, and delete apis Enhances index, get, and delete snapshot management apis May 19, 2022
@downsrob downsrob requested a review from bowenlan-amzn May 23, 2022 23:18
val smDoc = try {
parser(getResponse)
} catch (e: IllegalArgumentException) {
throw OpenSearchStatusException("Snapshot management doc could not be parsed", RestStatus.NOT_FOUND)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if 404 is the correct response code for a parsing exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, changed to INTERNAL_SERVER_ERROR. Do you think there is a better choice?

return smMetadata
}

suspend fun <T> Client.getSMDoc(docID: String, parser: (GetResponse) -> T): T {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used in multiple places or plan to be used in future for other things? Otherwise kind of seems like an over abstraction if it's just for the two above.. you might have ended up w/ even more lines of code than just explicitly implementing each above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the abstraction, it is a bit more code now but I don't think getSMDoc would have been used again, and now the exception messages can be more precise.

Signed-off-by: Clay Downs <[email protected]>
@downsrob downsrob requested a review from dbbaughe May 24, 2022 18:05
@downsrob downsrob merged commit 9d2b61a into opensearch-project:sm-dev May 24, 2022
@downsrob downsrob deleted the crud-enhancement branch May 24, 2022 21:22
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.

3 participants