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

Auto generate watchbot binaries when master is modified #235

Merged
merged 46 commits into from
Jul 5, 2018

Conversation

arunasank
Copy link
Contributor

@arunasank arunasank commented Jun 27, 2018

Fixes #202.

Next Actions

  • Tests - will involve a lot of mocks, more than testing the flow. Is it okay to not have tests for these?
  • Fix docs for the paths to the binaries

#228 will be nice to get out the door with this one, so that a squashed commit can generate a watchbot binary and make the state<>binary relationship really clear. Should we only build binaries when git tags are pushed, and not on every commit?

Note that the additional git clone ...ecs-watchbot...(master...code-pipeline#diff-e23bfe4bba63271ba8919f6fb8cfcd44R3) is because of a CodePipeline issue that does not give you the .git folder with your source code. I need this information to identify the sha being built, but more importantly the git tag information, since the sha can still be retrieved using process.env.CODEBUILD_RESOLVED_SOURCE_VERSION

cc/ @mapbox/platform-engine-room

@arunasank
Copy link
Contributor Author

  • Should we also generate builds for all commits on master starting from "♻️ this container"? For backwards-compatibility?

@arunasank
Copy link
Contributor Author

Should we also only build binaries on git tags, and not every git commit?

@jakepruitt
Copy link

Should we also only build binaries on git tags, and not every git commit?

I think git tags makes sense - that will make it easier for people to see the version in the URL they're using.

Copy link
Contributor

@rclark rclark left a comment

Choose a reason for hiding this comment

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

I believe that codepipeline + codebuild together should be able to handle downloading watchbot code and uploading build artifacts for you, without your having to write code to do it. Is there a reason the built-in artifact download/upload didn't work out for this use case?

},
Policies: [
{
PolicyName: cf.sub('BundlerPolicy${GitSha}'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you have the sha here? That will require the policy to change every time there's a deploy?

Copy link
Contributor Author

@arunasank arunasank Jun 28, 2018

Choose a reason for hiding this comment

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

Since the parameter is not used anywhere in the template, changing the gitsha would still cause cfn-config to give me a "No updates to perform" - This enables me to change the template in some way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to experiment with the RestartExecutionOnUpdate parameter in the CodePipeline resource to see if that helps here, since it seems Cloudformation specific and has no mention in the CodePipeline API.

Effect: 'Allow',
Action: [
's3:ListBucket',
's3:ListObjects',
Copy link
Contributor

Choose a reason for hiding this comment

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

s3:ListObjects is not a thing -- only s3:ListBucket

's3:ListBucket',
's3:ListObjects',
's3:GetObject',
's3:HeadObject',
Copy link
Contributor

Choose a reason for hiding this comment

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

s3:HeadObject also is not a thing -- covered by s3:GetObject

cd ecs-watchbot
npm ci --production
npm install -g pkg
pkg .
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't have to download watchbot code into your codebuild project. The whole point of using codepipeline is that when a commit is made to the repo, it should handle getting your code onto the codebuild project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The codebuild git project does not contain the .git folder associated with the repo, and I need this to retrieve the git tag information associated with my commit for example. More info in the opening comment of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per chat with Jake, another option is to retrieve the tag information from the package.json! Going to get rid of this, and read the tags from the package.json instead. : )

@arunasank arunasank force-pushed the code-pipeline branch 6 times, most recently from 3a0f720 to ff78d45 Compare June 29, 2018 05:26
Copy link

@jakepruitt jakepruitt left a comment

Choose a reason for hiding this comment

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

Looks like we need some tests, but other than that the approach here looks good from what I can tell!

@arunasank arunasank force-pushed the code-pipeline branch 2 times, most recently from 3f460de to f4a09ee Compare July 4, 2018 11:03
@arunasank arunasank merged commit 6105e2a into master Jul 5, 2018
@arunasank arunasank deleted the code-pipeline branch July 5, 2018 03:20
@arunasank arunasank mentioned this pull request Jul 5, 2018
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.

None yet

3 participants