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.

Sign up for GitHub

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

Jump to bottom

added eks-a-releaser files #8617

Open

ibix16 wants to merge 21 commits into aws:main from ibix16:eks-a-releaser-push

ibix16 commented Aug 15, 2024

Issue #, if available:

Description of changes:

Testing (if applicable):

Documentation added/planned (if applicable):

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Collaborator

eks-distro-bot commented Aug 15, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tatlat for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Collaborator

eks-distro-bot commented Aug 15, 2024

Hi @ibix16. Thanks for your PR.

I'm waiting for a aws member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

eks-distro-bot added needs-ok-to-test size/XXL labels

Member

abhinavmpandey08 commented Aug 15, 2024

/ok-to-test

eks-distro-bot added ok-to-test and removed needs-ok-to-test labels

abhinavmpandey08 reviewed

View reviewed changes

release/buildspecs/eks-a-releaser-buildspec/create-branch.yml Outdated

+              version: 0.2
+              env:
+                secrets-manager:
+                  SECRET_PAT: "Secret:PAT"

Member

abhinavmpandey08 Aug 15, 2024

Let's change this secret to an existing one in the build account

abhinavmpandey08 reviewed

View reviewed changes

release/buildspecs/eks-a-releaser-buildspec/create-branch.yml Outdated

+                pre_build:
+                  commands:
+                    - echo "Downloading compiled binary..."
+                    - aws s3 cp s3://eka-a-releaser-build-output/eks-a-releaser-build .

Member

abhinavmpandey08 Aug 15, 2024

Let's make the s3 bucket name configurable

ibix16 force-pushed the eks-a-releaser-push branch from 8e24aad to 126b86f Compare

August 15, 2024 22:58


          added eks-a-releaser files

e649269

ibix16 force-pushed the eks-a-releaser-push branch from 126b86f to e649269 Compare

August 15, 2024 23:16

Member

mitalipaygude commented Aug 16, 2024 •

edited

Loading

Let's update the description section with details about the PR and testing.

Also this is a very large PR to review. We might end up missing things while reviewing. Can we break these PRs up into smaller PRs?

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-branch.go Outdated

+              	if RELEASE_TYPE == "minor" {
+              		err := createMinorBranches()
+              		if err != nil {
+              			fmt.Printf("error calling createMinorBranches %s", err)

Member

mitalipaygude Aug 16, 2024

Let's return these errors to the caller instead of using Printf statements throughout.

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-branch.go Outdated

+              	// else
+              	err := createPatchBranch()
+              	if err != nil {
+              		fmt.Printf("error calling createPatchBranch %s", err)

Member

mitalipaygude Aug 16, 2024

Same comment about printing errors.

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-release.go Outdated

+              	// else
+              	//create client
+              	accessToken := os.Getenv("SECRET_PAT")

Member

mitalipaygude Aug 16, 2024

nit: we can define this as a constant instead of writing the string value at multiple places.

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-release.go Outdated

+              	return rel, nil
+              }
+              func retrieveLatestProdCLIHash(releaseType string) string {

Member

mitalipaygude Aug 16, 2024

Suggested change

      
            func retrieveLatestProdCLIHash(releaseType string) string {
          
            func retrieveLatestProdCLIHash(releaseType string) error {

Let's return error instead of the string

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-release.go Outdated

+              		commits, _, err := client.Repositories.ListCommits(ctx, usersForkedRepoAccount, EKSAnyrepoName, opts)
+              		if err != nil {
+              			return "error fetching commits list"

Member

mitalipaygude Aug 16, 2024

Suggested change

      
            			return "error fetching commits list"
          
            			return fmt.Errorf("fetching commits list %v", err)

something like this

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-release.go Outdated


		func createGitHubRelease(releaseTag github.RepositoryRelease) (github.RepositoryRelease, error) {

		latestVersionValue := os.Getenv("LATEST_VERSION")

Member

mitalipaygude Aug 16, 2024

nit: constant for LATEST_VERSION

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-release.go Outdated

+              	ctx := context.Background()
+              	client := github.NewClient(nil).WithAuthToken(accessToken)
+              	release, _, err := client.Repositories.GetReleaseByTag(ctx, upStreamRepoOwner, EKSAnyrepoName, latestVersionValue)

Member

mitalipaygude Aug 16, 2024

Shouldn't we also return here if we encounter an error?

Suggested change

      
            	release, _, err := client.Repositories.GetReleaseByTag(ctx, upStreamRepoOwner, EKSAnyrepoName, latestVersionValue)
          
            	release, _, err := client.Repositories.GetReleaseByTag(ctx, upStreamRepoOwner, EKSAnyrepoName, latestVersionValue)
          
            	if err != nil {
          
            		return nil, err
          
            	}

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/create-release.go Outdated

+              	release, _, err := client.Repositories.GetReleaseByTag(ctx, upStreamRepoOwner, EKSAnyrepoName, latestVersionValue)
+              	if err == nil {
+              		fmt.Printf("Release %s already exists!\n", latestVersionValue)

Member

mitalipaygude Aug 16, 2024

I would recommend logging these statements rather than printing them.

mitalipaygude reviewed

View reviewed changes

release/cli/cmd/prod-bundle.go

+              	pr, _, err := client.PullRequests.Create(ctx, upStreamRepoOwner, EKSAnyrepoName, newPR)
+              	if err != nil {
+              		return fmt.Errorf("error creating PR %s", err)

Member

mitalipaygude Aug 16, 2024 •

edited

Loading

Suggested change

      
            		return fmt.Errorf("error creating PR %s", err)
          
            		return fmt.Errorf("creating PR %v", err)

ibix16 added 8 commits

August 19, 2024 13:16


          Update go-build-binary.yml

4c4a633


          Update go-build-binary.yml

98146b4


          Update go-build-binary.yml

ea696d0


          Update create-branch.go

99cc208


          Update create-release.go

af7bc68


          Update prod-bundle.go

a8de664


          Update prod-cli.go

3af8b06


          Update stage-bundle.go

96e94b1

ibix16 added 12 commits

August 19, 2024 13:35


          Update stage-cli.go

77274ec


          Update update-homebrew.go

35093a1


          Update update-makefile.go

0e734de


          Update update-prow.go

78e7126


          Update go.mod

ab6cc3d


          Update go-build-binary.yml

86bfcf7


          Update go-build-binary.yml

779b0f6


          Update go.mod

6da1f67


          Update go-build-binary.yml

7f16939


          Update go-build-binary.yml

8f8e575


          Update go-build-binary.yml

3636b7a


          Update create-release.go

1d8bb12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test size/XXL