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

Remove template key requirement from composable template param source #1486

Closed
wants to merge 1 commit into from

Conversation

DJRickyB
Copy link
Contributor

@DJRickyB DJRickyB commented May 9, 2022

According to the API, template is not a required key in the create request. This assumption caused valid composable templates to get skipped when using the native create-composable-template task.

@DJRickyB DJRickyB added the bug Something's wrong label May 9, 2022
@DJRickyB DJRickyB added this to the 2.5.0 milestone May 9, 2022
@DJRickyB DJRickyB requested review from pquentin and inqueue May 9, 2022 17:53
@DJRickyB DJRickyB self-assigned this May 9, 2022
Copy link
Member

@inqueue inqueue left a comment

Choose a reason for hiding this comment

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

I tested current behavior, without this PR, to see what Rally does with a composable index template that is missing a template object, and contains a composed_of array with a single component template:

{
  "name": "create-template-test",
  "operation-type": "create-composable-template",
  "template": "metricbeat-template-1",
  "body": {
    "index_patterns": [
      "metricbeat"
    ],
    "composed_of": [
      "metricbeat-component-1"
    ]
  }
}

A create-component-template operation was also added:

{
  "name": "create-template-metricbeat-component-1",
  "operation-type": "create-component-template",
  "template": "metricbeat-component-1",
  "body": {
    "template": {
      "settings": {
        "number_of_shards": 1
      },
      "mappings": {
        "_source": {
          "enabled": false
        }
      }
    }
  }
}

As documented in the 'Note' section, these operations should be executed in order of create-component-template, then create-composable-template to ensure any component templates referenced by the composable template exist prior to creating the composable template.

I found Rally to create both the component and composable templates correctly when the challenges are specified in the correct order. However, Rally unexpectedly continues when the composable template is first, followed by the component template. Additionally, there is no sign of the 400 error it should have received when creating a composable template referencing a non-existent component template. Rally should not be allowed to continue in the case where creating the index template (composable) results in a 400 error code:

PUT /_index_template/metricbeat-template-1
{
  "error": {
    "root_cause": [
      {
        "type": "invalid_index_template_exception",
        "reason": "index_template [metricbeat-template-1] invalid, cause [index template [metricbeat-template-1] specifies component templates [metricbeat-component-1] that do not exist]"
      }
    ],
    "type": "invalid_index_template_exception",
    "reason": "index_template [metricbeat-template-1] invalid, cause [index template [metricbeat-template-1] specifies component templates [metricbeat-component-1] that do not exist]"
  },
  "status": 400
}

Line 447 appears to be checking for the template/settings path to apply settings to the template, and not as a form of validation for the template itself.

This assumption caused valid composable templates to get skipped when using the native create-composable-template task.

Could the issue have been somewhere else, like in challenge ordering?

@DJRickyB
Copy link
Contributor Author

DJRickyB commented May 9, 2022 via email

@DJRickyB
Copy link
Contributor Author

closing as insufficient. opened #1487 to track the issue

@DJRickyB DJRickyB closed this May 10, 2022
@DJRickyB DJRickyB deleted the optional_template branch May 10, 2022 21:57
@cavokz
Copy link
Contributor

cavokz commented May 12, 2022

With the following track (composable template referring to a non existent component template) I get the 400 error as expected:

{
  "version": 2,
  "description": "Dummy track",
  "schedule": [
    {
      "operation": {
        "name": "create-template-dummy-component-1",
        "operation-type": "create-component-template",
        "template": "dummy-component-1",
        "body": {
          "template": {
            "settings": {
              "number_of_shards": 1
            },
            "mappings": {
              "_source": {
                "enabled": false
              }
            }
          }
        }
      }
    },
    {
      "operation": {
        "name": "create-template-dummy",
        "operation-type": "create-composable-template",
        "template": "dummy-template-1",
        "body": {
          "index_patterns": [
            "dummy-*"
          ],
          "template": {
          },
          "composed_of": [
            "dummy-component-2"
          ]
        }
      }
    }
  ]
}

Also with this other track (composable created before the component) I get 400 as expected:

{
  "version": 2,
  "description": "Dummy track",
  "schedule": [
    {
      "operation": {
        "name": "create-template-dummy",
        "operation-type": "create-composable-template",
        "template": "dummy-template-1",
        "body": {
          "index_patterns": [
            "dummy-*"
          ],
          "template": {
          },
          "composed_of": [
            "dummy-component-1"
          ]
        }
      }
    },
    {
      "operation": {
        "name": "create-template-dummy-component-1",
        "operation-type": "create-component-template",
        "template": "dummy-component-1",
        "body": {
          "template": {
            "settings": {
              "number_of_shards": 1
            },
            "mappings": {
              "_source": {
                "enabled": false
              }
            }
          }
        }
      }
    }
  ]
}

@DJRickyB
Copy link
Contributor Author

Note that this change only influences templates provided by the top-level track field composable-templates, and not an explicit body in the task. Also note, any further discussion should go to the root issue #1487

@pquentin pquentin changed the title remove template key requirement from composable template param source Remove template key requirement from composable template param source Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants