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

upgrade aws-sdk-go-v2 versions #5196

Closed
wants to merge 2 commits into from

Conversation

tom-256
Copy link

@tom-256 tom-256 commented Sep 6, 2024

What this PR does / why we need it:
For enable restart policy in ECS task definitions.

Which issue(s) this PR fixes:

Fixes #5144

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

I have confirmed that the ECS restart policy is applied correctly with the following YAML configuration.

containerDefinitions:
  - name: test
...
    restartPolicy:
      enabled: true
      ignoredExitCodes:
        - 0
      restartAttemptPeriod: 180

The resulting ECS task definition JSON has been registered as follows:

{
...
    "containerDefinitions": [
        {
            "name": "test",
...
            "restartPolicy": {
                "enabled": true,
                "ignoredExitCodes": [
                    0
                ],
                "restartAttemptPeriod": 180
            },
        }
    ],
...
}

khanhtc1202
khanhtc1202 previously approved these changes Sep 6, 2024
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Thank you 🚀

@khanhtc1202
Copy link
Member

@tom-256 please sign the commit follow failed CI instructions 👀

@t-kikuc t-kikuc changed the title update aws-sdk version update aws-sdk version from v1.17.7 to v1.30.4 Sep 6, 2024
@t-kikuc t-kikuc changed the title update aws-sdk version from v1.17.7 to v1.30.4 update aws-sdk-go-v2 version from v1.17.7 to v1.30.4 and aws-sdk-go-v2/service/ecs version from v1.24.2 to v1.45.1 Sep 6, 2024
@t-kikuc t-kikuc changed the title update aws-sdk-go-v2 version from v1.17.7 to v1.30.4 and aws-sdk-go-v2/service/ecs version from v1.24.2 to v1.45.1 update aws-sdk-go-v2/service/ecs version from v1.24.2 to v1.45.1 Sep 6, 2024
Copy link
Member

@t-kikuc t-kikuc left a comment

Choose a reason for hiding this comment

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

@tom-256
Would you fix https://github.com/pipe-cd/pipecd/blob/master/pkg/app/piped/platformprovider/ecs/diff_test.go#L105 as below?

from:

@@ -17,7 +17,7 @@

to:

@@ -18,7 +18,7 @@

That's because the test failed
since the new field RestartPolicy is added and the row number is slided.

Signed-off-by: tom-256 <[email protected]>
Signed-off-by: tom-256 <[email protected]>
@tom-256 tom-256 marked this pull request as ready for review September 11, 2024 09:58
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Nice, thank you 👍

@t-kikuc
Copy link
Member

t-kikuc commented Sep 12, 2024

Thank you so much,
I'll test a little by myself and approve!

@t-kikuc
Copy link
Member

t-kikuc commented Sep 13, 2024

@tom-256

It turned out that we need to upgrade all aws-sdk-go-v2 modules, not only ecs.
That's due to breaking changes of the SDK...

Without that, API calls return not found, ResolveEndpointV2 errors. (especially, ELB and Lambda)

To fix it, would you follow the steps below?

1. upgrade all aws-sdk-go-v2 modules

go get -u github.com/aws/aws-sdk-go-v2/...

cf. aws/aws-sdk-go-v2#2370 (comment)

2. fix compile error(s)

e.g.

should be:

if *rule.IsDefault {

This is due to a breaking change aws/aws-sdk-go-v2#2333.

@t-kikuc t-kikuc changed the title update aws-sdk-go-v2/service/ecs version from v1.24.2 to v1.45.1 upgrade aws-sdk-go-v2 versions Sep 17, 2024
@t-kikuc
Copy link
Member

t-kikuc commented Sep 27, 2024

I'll continue on #5241

@t-kikuc t-kikuc closed this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable restart policy property for ECS application
3 participants