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

Adding support for us-isof and eu-isoe regions #8151

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Adding support for us-isof and eu-isoe regions #8151

merged 1 commit into from
Jan 22, 2025

Conversation

TiberiuGC
Copy link
Contributor

Description

Closes #8095

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@TiberiuGC TiberiuGC added the kind/feature New feature or request label Jan 21, 2025
@TiberiuGC
Copy link
Contributor Author

Thanks you for this contribution @jdwtf !

Unfortunately I couldn't rebase your PR directly likely because of a GH bug... so I've opened another one with your commit.

@@ -101,7 +101,7 @@ func dumpLogsToDisk(logBuffer *bytes.Buffer, errorString string) error {
if _, err := os.Stat("logs/"); os.IsNotExist(err) {

if err := os.Mkdir("logs/", 0755); err != nil {
return fmt.Errorf(err.Error())
return fmt.Errorf("%s", err.Error())
Copy link
Member

Choose a reason for hiding this comment

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

qq: why not just return the err? and same question for similar changes below

Copy link

Choose a reason for hiding this comment

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

I was trying to stay light touch. The linter was complaining about no string format on the Errorf call so I just gave it one. The description of my original PR (that I had to cancel) has much better detail:

Description

Adding support for new isolated AWS regions to include necessary changes for testing and fixing linter errors/complaints. A prototype binary was generated with these changes and we are able to successfully test functionality in region.

Key Changes For Build:

  • pkg/apis/eksctl.io/v1alpha5/partitions.go: added
    • added logic to account for the four service endpoints that have non-standard domain prefixes in these partitions
    • partitions strings
    • partitions objects with region-appropriate
      • service mappings
      • domain prefixes
    • added partitions to ISO part of GetEndpointServiceDomainPrefix
  • pkg/apis/eksctl.io/v1alpha5/types.go: added
    • in-region resource account IDs
    • region strings
    • SupportedRegions strings

Key Changes For Testing:

  • pkg/actions/nodegroup/testdata all *.json files updated ServicePrincipalPartitionMap
  • pkg/cfn/builder/karpenter_test.go updated expectedTemplate* template strings
  • pkg/cfn/builder/testdata/nodegroup_access_entry all *.json files ServicePrincipalPartitionMap
  • pkg/cfn/builder/testdata/ added service_details_isob.json, service_details_isoe.json, service_details_isof.json
  • pkg/cfn/builder/vpc_endpoint_test.go to add tests for isob, isoe and isof

Documentation:

  • added new regions' information to
    • userdocs/theme/home.html
    • userdocs/src/getting-started.md

Linter Warnings/Errors:

  • integration/tests/crud/creategetdelete_test.go: error and return value check for Sscanf
  • added static string formatting to
    • cmd/eksctl/logger.go
    • pkg/actions/accessentry/task.go
    • pkg/actions/addon/tasks.go
    • pkg/actions/anywhere/anywhere.go
    • pkg/actions/podidentityassociation/tasks.go
    • pkg/cfn/template/matchers/matchers.go
    • pkg/eks/api_test.go
    • pkg/ssh/client/ssh.go
  • unnecessary nil checks for 0 length slice
    • pkg/apis/eksctl.io/v1alpha5/addon.go
    • pkg/apis/eksctl.io/v1alpha5/addon.go

Copy link
Contributor Author

@TiberiuGC TiberiuGC Jan 22, 2025

Choose a reason for hiding this comment

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

updated this line to just return err, the other similar changes seem correct to me

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

👍

@TiberiuGC TiberiuGC merged commit 48f1e72 into main Jan 22, 2025
10 checks passed
@TiberiuGC TiberiuGC deleted the jdwtf-main branch January 22, 2025 15:55
TiberiuGC added a commit to TiberiuGC/eksctl that referenced this pull request Jan 22, 2025
adding support for us-isof and eu-isoe regions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] add support for us-isof and eu-isoe regions
3 participants