Skip to content

Adjusting deploy spec#49

Merged
ycombinator merged 3 commits intoelastic:masterfrom
ycombinator:deploy-spec
Sep 18, 2020
Merged

Adjusting deploy spec#49
ycombinator merged 3 commits intoelastic:masterfrom
ycombinator:deploy-spec

Conversation

@ycombinator
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator commented Sep 18, 2020

This PR adjusts the spec for the _dev/deploy folder as per this proposal: elastic/integrations#263 (comment).

With this PR the spec will expect a specific folder name under _dev/deploy corresponding to a supported deployment model. So a package could have this:

_dev/
  deploy/
   variants.yml (optional)
    docker/
      docker-compose.yml    (required)
      Dockerfile   (optional)
      (other, optional files as needed via `additionalContents: true`)

or:

_dev/
  deploy/
    terraform/
      TBD

or:

_dev/
  deploy/
    SOME_NEW_MODEL/
      TBD

For now, only the Docker-based deployment model is supported. The spec changes in this PR reflect this. In the future when we add other deployment models, we'll need to adjust the spec accordingly, e.g. to expect that there must be exactly one sub-folder under _dev/deploy since a package should only specify one deployment model.

@ycombinator ycombinator requested a review from mtojek September 18, 2020 11:27
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Sep 18, 2020 •
edited
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #49 updated]

  • Start Time: 2020-09-18T12:11:11.646+0000

  • Duration: 2 min 8 sec

@ycombinator
Copy link
Copy Markdown
Contributor Author

ycombinator commented Sep 18, 2020 •
edited
Loading

I would expect CI to fail on this PR as it stands. Specifically, I'd expect this test case to fail as the mock package for that test case has a _dev/deploy folder but no docker folder under it:

package-spec/code/go/pkg/validator/validator_test.go

Line 19 in d669841

"bad_deploy_variants": {

I wonder why CI is passing on this test case 🤔 . I'm going to investigate this before I push a commit to fix the expected test failure in this PR.

@ycombinator
Copy link
Copy Markdown
Contributor Author

ycombinator commented Sep 18, 2020

Okay, turns out I had accidentally pushed up the test fix commits already, which is why CI was passing earlier. I temporarily removed them now and CI is failing as expected. Phew!

So just PEBCAK, nothing to see here 🙈 .

I will push up the test fix commits again now.

@ycombinator ycombinator merged commit a193a34 into elastic:master Sep 18, 2020
@ycombinator ycombinator deleted the deploy-spec branch September 18, 2020 12:42
@ycombinator ycombinator mentioned this pull request Sep 18, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mtojek mtojek mtojek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ycombinator @elasticmachine @mtojek