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

Update aws-go-sdk to v1.47.11 to support EKS Pod Identity #5796

Merged

Conversation

mthemis-provenir
Copy link
Contributor

@mthemis-provenir mthemis-provenir commented Oct 1, 2024

Tracking issue

Closes #5794

Why are the changes needed?

Updates the AWS Go SDK to support EKS Pod Identity, a newer, more convenient way of authenticating service accounts to IAM roles.

What changes were proposed in this pull request?

To bring the AWS Go SDK up to the minimum required to support the feature.

How was this patch tested?

I tried to test the various modules however I kept getting the same error, so I then checked out the latest release (v1.13.1), reran everything and it failed at the same place, which makes me think it's something wrong with my setup. I did have to edit the PostgreSQL and Minio deployments because they aren't compatible with arm64, so it might be due to that. I'd very much appreciate it if anyone with a working environment could give this a quick test.

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

N/A

Docs link

None added, but can look to add documentation if desired.

Copy link

welcome bot commented Oct 1, 2024

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@mthemis-provenir
Copy link
Contributor Author

I didn't want to contaminate the OP, but for the interested, this is the same place testing failed on, regardless of my changes or latest release.

2024/10/01 22:56:30 /home/matt/source/thirdparty/flyte/flyteadmin/tests/bootstrap.go:118 ERROR: relation "admin_tags" does not exist (SQLSTATE 42P01)
[6.087ms] [rows:0] TRUNCATE TABLE admin_tags;

2024/10/01 22:56:30 /home/matt/source/thirdparty/flyte/flyteadmin/tests/bootstrap.go:119 ERROR: relation "execution_admin_tags" does not exist (SQLSTATE 42P01)
[6.204ms] [rows:0] TRUNCATE TABLE execution_admin_tags;

2024/10/01 22:56:30 /home/matt/source/thirdparty/flyte/flyteadmin/tests/bootstrap.go:120
[26.574ms] [rows:0] TRUNCATE TABLE execution_tags;
=== RUN   TestGetWorkflows/TestGetWorkflowGrpc
=== RUN   TestGetWorkflows/TestGetWorkflowHTTP
=== RUN   TestGetWorkflows/TestListWorkflowGrpc
=== RUN   TestGetWorkflows/TestListWorkflowHTTP
=== RUN   TestGetWorkflows/TestListWorkflow_PaginationGrpc
=== RUN   TestGetWorkflows/TestListWorkflow_PaginationHTTP
=== RUN   TestGetWorkflows/TestListWorkflow_FiltersGrpc
=== RUN   TestGetWorkflows/TestListWorkflow_FiltersHTTP
--- PASS: TestGetWorkflows (0.85s)
    --- PASS: TestGetWorkflows/TestGetWorkflowGrpc (0.00s)
    --- PASS: TestGetWorkflows/TestGetWorkflowHTTP (0.00s)
    --- PASS: TestGetWorkflows/TestListWorkflowGrpc (0.01s)
    --- PASS: TestGetWorkflows/TestListWorkflowHTTP (0.00s)
    --- PASS: TestGetWorkflows/TestListWorkflow_PaginationGrpc (0.00s)
    --- PASS: TestGetWorkflows/TestListWorkflow_PaginationHTTP (0.00s)
    --- PASS: TestGetWorkflows/TestListWorkflow_FiltersGrpc (0.00s)
    --- PASS: TestGetWorkflows/TestListWorkflow_FiltersHTTP (0.00s)
FAIL
FAIL	github.com/flyteorg/flyte/flyteadmin/tests	139.698s
FAIL
make: *** [Makefile:29: integration] Error 1

@Sovietaced
Copy link
Contributor

I didn't want to contaminate the OP, but for the interested, this is the same place testing failed on, regardless of my changes or latest release.

I just kicked off the CI so we can see if it fails the unit tests there.

Copy link

codecov bot commented Oct 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.35%. Comparing base (66ff152) to head (7ede8a6).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5796      +/-   ##
==========================================
+ Coverage   36.31%   36.35%   +0.03%     
==========================================
  Files        1304     1304              
  Lines      110048   110147      +99     
==========================================
+ Hits        39964    40042      +78     
- Misses      65928    65938      +10     
- Partials     4156     4167      +11     
Flag Coverage Δ
unittests-datacatalog 51.37% <ø> (ø)
unittests-flyteadmin 55.60% <ø> (+0.01%) ⬆️
unittests-flytecopilot 12.17% <ø> (ø)
unittests-flytectl 62.21% <ø> (ø)
unittests-flyteidl 7.17% <ø> (+0.04%) ⬆️
unittests-flyteplugins 53.35% <ø> (ø)
unittests-flytepropeller 42.02% <ø> (+0.09%) ⬆️
unittests-flytestdlib 55.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mthemis-provenir
Copy link
Contributor Author

I didn't want to contaminate the OP, but for the interested, this is the same place testing failed on, regardless of my changes or latest release.

I just kicked off the CI so we can see if it fails the unit tests there.

Thanks for that. Some of the failures are related to missing an update in flytectl which I've corrected, but the other failures I'm not really sure how they could relate to my updates.

@mthemis-provenir
Copy link
Contributor Author

@Sovietaced It looks like it passed all the required tests, and the only failures seem to be related to code coverage which are unrelated to my changes. Any chance this gets an approval and merge? Would be nice to get this in for the next release.

@Sovietaced
Copy link
Contributor

@Sovietaced It looks like it passed all the required tests, and the only failures seem to be related to code coverage which are unrelated to my changes. Any chance this gets an approval and merge? Would be nice to get this in for the next release.

I'm going to take a look at the unit test failure. codecov usually fails until all the actions run and then it passes

@Sovietaced
Copy link
Contributor

I'm going to take a look at the unit test failure. codecov usually fails until all the actions run and then it passes

I re-ran the failed test and it passed. Not sure what happened.

I'm actually just a committer and although I can merge this I'd like to connect with the Flyte folks to see if I'm missing anything.

@Sovietaced Sovietaced added the dependencies Pull requests that update a dependency file label Oct 6, 2024
@eapolinario eapolinario merged commit b9900fe into flyteorg:master Oct 7, 2024
53 checks passed
siiddhantt pushed a commit to siiddhantt/flyte that referenced this pull request Oct 7, 2024
davidmirror-ops pushed a commit that referenced this pull request Oct 31, 2024
* Update registering_workflows.md

Signed-off-by: Siddhant Rai <[email protected]>

* feat: note in registering_workflows for k8sPod.dataConfig fix

Signed-off-by: Siddhant Rai <[email protected]>

* update: remove note as requested change

Signed-off-by: Siddhant Rai <[email protected]>
Signed-off-by: Siddhant Rai <[email protected]>

* Flyte docs overhaul (phase 1) (#5772)

* move flytekit and flytectl docs into API section

Signed-off-by: nikki everett <[email protected]>

* switch to docsearch module and env variables

Signed-off-by: nikki everett <[email protected]>

* reorganize content for pydata theme

Signed-off-by: nikki everett <[email protected]>

* more docs reorganization

Signed-off-by: nikki everett <[email protected]>

* switch to pydata theme

Signed-off-by: nikki everett <[email protected]>

* reorganize concepts/glossary and ecosystem docs

Signed-off-by: nikki everett <[email protected]>

* remove unneeded custom CSS and JS files

Signed-off-by: nikki everett <[email protected]>

* add redirects

Signed-off-by: nikki everett <[email protected]>

* add more redirects

Signed-off-by: nikki everett <[email protected]>

* first pass at updating docs contributing guide

Signed-off-by: nikki everett <[email protected]>

* remove core use cases

Signed-off-by: nikki everett <[email protected]>

* more edits to docs contributing guide

Signed-off-by: nikki everett <[email protected]>

* more edits to the flytesnacks contributing guide

Signed-off-by: nikki everett <[email protected]>

* add content to API reference index page, use consistent title and format for API reference section titles

Signed-off-by: nikki everett <[email protected]>

* rename deployment section

Signed-off-by: nikki everett <[email protected]>

* reorganize sections

Signed-off-by: nikki everett <[email protected]>

* fix typos

Signed-off-by: nikki everett <[email protected]>

* add docsearch index name and app id

Signed-off-by: nikki everett <[email protected]>

* add ref to docs contributing doc and move all docsearch stuff to env vars again

Signed-off-by: nikki everett <[email protected]>

* docs overhaul: render flyteidl under the /api/ path (#5802)

* fix flyteidl structure so it renders under /api/

Signed-off-by: Niels Bantilan <[email protected]>

* do not check in flyteidl docs

Signed-off-by: Niels Bantilan <[email protected]>

* update gitignore and unneeded conf

Signed-off-by: Niels Bantilan <[email protected]>

* add mock DOCSEARCH_API_KEY to docs test ci

Signed-off-by: Niels Bantilan <[email protected]>

* add css styling (#5803)

* add css styling

Signed-off-by: Niels Bantilan <[email protected]>

* update logo height

Signed-off-by: Niels Bantilan <[email protected]>

---------

Signed-off-by: Niels Bantilan <[email protected]>

---------

Signed-off-by: Niels Bantilan <[email protected]>

* use same icon as union docs

Signed-off-by: nikki everett <[email protected]>

* sp error

Signed-off-by: Niels Bantilan <[email protected]>

---------

Signed-off-by: nikki everett <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Co-authored-by: Niels Bantilan <[email protected]>
Signed-off-by: Siddhant Rai <[email protected]>

* [Flyte][3][flytepropeller][Attribute Access][flytectl] Binary IDL With MessagePack (#5763)

Signed-off-by: Siddhant Rai <[email protected]>

* Update aws-go-sdk to v1.47.11 to support EKS Pod Identity (#5796)

Signed-off-by: Siddhant Rai <[email protected]>

* Update docs/user_guide/flyte_fundamentals/registering_workflows.md

Co-authored-by: Nikki Everett <[email protected]>
Signed-off-by: Siddhant Rai <[email protected]>

* Update registering_workflows.md

Signed-off-by: Siddhant Rai <[email protected]>

---------

Signed-off-by: Siddhant Rai <[email protected]>
Signed-off-by: Siddhant Rai <[email protected]>
Signed-off-by: nikki everett <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Co-authored-by: Nikki Everett <[email protected]>
Co-authored-by: Niels Bantilan <[email protected]>
Co-authored-by: Future-Outlier <[email protected]>
Co-authored-by: mthemis-provenir <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Housekeeping] Update aws-go-sdk to support EKS Pod Identity
3 participants