-
Notifications
You must be signed in to change notification settings - Fork 366
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
Fix code_deploy to properly register S3 rev #732
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the specs, and add build log URL(s) that shows deployment(s) using this code.
Thanks.
How do I run the specs? I guess I need to start by Build log: https://travis-ci.com/TeliaSoneraNorge/rapidshop-b2b/builds/63097181 |
You can see the failure here: https://travis-ci.org/travis-ci/dpl/jobs/326692313#L1686 |
Also, the build you indicated used the release 1.8.43, not your code. Could you review https://github.com/travis-ci/dpl/blob/master/TESTING.md and try again? Thanks. |
I will. Can you please tell me how to run specs locally? As described
above, bundle indtall fails :-(
ons. 10. jan. 2018 kl. 19.52 skrev Hiro Asari <notifications@github.com>:
… Also, the build you indicated used the release 1.8.43, not your code.
Could you review https://github.com/travis-ci/dpl/blob/master/TESTING.md
and try again? Thanks.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#732 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAmJPip4txYRt3wnDKXkXqYKwZ0R8Uy7ks5tJQb0gaJpZM4RW2_->
.
|
c24e64e
to
3667bc9
Compare
9e406e3
to
f6fd837
Compare
Here is build log for the latest version: https://travis-ci.com/TeliaSoneraNorge/rapidshop-b2b/builds/63190665 Is there anything else I should fix? PS: I would still appreciate instructions for how to get bundle and rspec to run locally. (As mentioned above, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over all it seems good, but I have some stylistic suggestions and one question about unclear piece of data.
lib/dpl/provider/code_deploy.rb
Outdated
@@ -61,6 +76,16 @@ def github_revision | |||
end | |||
|
|||
def push_app | |||
rev = revision() | |||
if rev[:s3_location] | |||
revInfo = rev[:s3_location] |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
lib/dpl/provider/code_deploy.rb
Outdated
end | ||
code_deploy.register_application_revision({ | ||
revision: rev, | ||
application_name: options[:application] || option(:application_name), |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
lib/dpl/provider/code_deploy.rb
Outdated
s3obj = s3api.get_object({ | ||
bucket: option(:bucket), | ||
key: s3_key, | ||
range: "bytes=0-1" |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
spec/provider/code_deploy_spec.rb
Outdated
@@ -89,7 +89,9 @@ | |||
s3_location: { | |||
bucket: 'bucket', | |||
bundle_type: 'tar', | |||
key: 'key' | |||
key: 'key', | |||
version: 'ObjectVersionId', |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Thanks, will fix. I still wait for instructions how to run rspec locally
(see bundle install issue mentioned above) :-)
tir. 16. jan. 2018 kl. 15:45 skrev Hiro Asari <notifications@github.com>:
… ***@***.**** requested changes on this pull request.
Over all it seems good, but I have some stylistic suggestions and one
question about unclear piece of data.
------------------------------
In lib/dpl/provider/code_deploy.rb
<#732 (comment)>:
> @@ -61,6 +76,16 @@ def github_revision
end
def push_app
+ rev = revision()
+ if rev[:s3_location]
+ revInfo = rev[:s3_location]
rev_info
------------------------------
In lib/dpl/provider/code_deploy.rb
<#732 (comment)>:
> @@ -61,6 +76,16 @@ def github_revision
end
def push_app
+ rev = revision()
+ if rev[:s3_location]
+ revInfo = rev[:s3_location]
+ log "Registering app revision with version=#{revInfo[:version]}, etag=#{revInfo[:e_tag]}"
+ end
+ code_deploy.register_application_revision({
+ revision: rev,
+ application_name: options[:application] || option(:application_name),
We can squash the spaces around ||.
------------------------------
In lib/dpl/provider/code_deploy.rb
<#732 (comment)>:
> @@ -39,13 +43,24 @@ def revision
end
end
+ def revision_version_info
+ s3obj = s3api.get_object({
+ bucket: option(:bucket),
+ key: s3_key,
+ range: "bytes=0-1"
Is there a reason for this value? Why is it hard coded?
------------------------------
In spec/provider/code_deploy_spec.rb
<#732 (comment)>:
> @@ -89,7 +89,9 @@
s3_location: {
bucket: 'bucket',
bundle_type: 'tar',
- key: 'key'
+ key: 'key',
+ version: 'ObjectVersionId',
Use object_version_id, and etag. The names are inconsequential here, so
lower the cost of skimming the code by mimicking other constants in this
bit.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#732 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAmJPryvxUXbflQdDiXZc-SJD9lLq7LHks5tLLYYgaJpZM4RW2_->
.
|
11cebf9
to
7eac080
Compare
Register a revision based on the latest (presumabely just uploaded) file when using S3 or the given commit when using GH so that CodeDeploy will deploy the correct archive. Otherwise it would deploy the latest _registered revision_, not the latest _uploaded archive_. As [AWS the documentation says](https://docs.aws.amazon.com/codedeploy/latest/userguide/application-revisions-register.html): > If you've already called the push command to push an application > revision to Amazon S3, you don't need to register the revision. > However, if you upload a revision to Amazon S3 through other means and > want the revision to appear in the AWS CodeDeploy console or through > the AWS CLI, follow these steps to register the revision first. > If you've pushed an application revision to a GitHub repository and > want the revision to appear in the AWS CodeDeploy console or through > the AWS CLI, you must also follow these steps. Tested both with versioned and non-versioned (no version, still has etag) buckets.
Here is build log for the latest deployer update: https://travis-ci.com/TeliaSoneraNorge/rapidshop-b2b/builds/63617699 |
|
That wasnt really helpful. As described above, I have tried to use a
Vagrantbmachine but ‘bundle install’ failed due to issues with json-1.8.1.
What Docker image do you use? How do tou mitigate this failure?
tor. 18. jan. 2018 kl. 21:22 skrev Hiro Asari <notifications@github.com>:
… rake should run specs. However, I suggest using a Docker image or some
other means of separation, as it may perform some system-level changes.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#732 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAmJPtQPuACBaChVScxXrndsvEwte44Xks5tL6gZgaJpZM4RW2_->
.
|
Can you guys rerun travis? The single version of ruby that failed as an empty log so it is hard to troubleshoot. Perhaps just a temporary hitch? |
@BanzaiMan I think I have addressed all you wanted? Can this be merged now? Thanks! |
Thank you! |
Thank you for the help and patience! (Though I would still appreciate also advice on how to Any idea when will this be released and availablie at travis-ci? Thanks! |
I'm thinking of (hopefully) the last 1.8.x release tomorrow. |
Register a revision based on the latest (presumabely just uploaded)
file (when using S3) so that CodeDeploy will deploy the correct archive.
Otherwise it would deploy the latest registered revision, not the
latest uploaded archive.
As AWS the documentation
says:
Notice that we should also do this for GitHub:
but I do not have the capacity to do that.
Tested both with versioned and non-versioned (no version, still has
etag) buckets.