Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Remove duplicate download go func#6026

Merged
zrhoffman merged 21 commits intoapache:masterfrom
bueale:remove-duplicate-download-go-func
Jul 23, 2021
Merged

Remove duplicate download go func#6026
zrhoffman merged 21 commits intoapache:masterfrom
bueale:remove-duplicate-download-go-func

Conversation

@bueale
Copy link
Copy Markdown
Contributor

@bueale bueale commented Jul 14, 2021

What does this PR (Pull Request) do?

Which Traffic Control components are affected by this PR?

  • CI tests

What is the best way to verify this PR?

If this is a bug fix, what versions of Traffic Control are affected?

The following criteria are ALL met by this PR

  • This PR does not require tests
  • This PR does not require necessary documentation updates
  • This PR includes No necessary changes to CHANGELOG.md
  • This PR includes any and all required license headers
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@zrhoffman
Copy link
Copy Markdown
Member

zrhoffman commented Jul 15, 2021

After removing the GOROOT lines in a2e301d, go-test action from the Go Unit Tests workflow run does not have Go, because it runs in a Docker container. download_go should be re-added to go-test. As for the others, they run directly on the GitHub runner, which already has Go from the GitHub Actions Ubuntu2004 virtual environment.

Not sure why the Traffic Ops Go client/API integration tests / API_tests and TP Integration Tests / TP_Integration_tests workflows are failing.

@zrhoffman
Copy link
Copy Markdown
Member

The Traffic Ops Go client/API integration tests / API_tests and TP Integration Tests / TP_Integration_tests workflows fail because they both use the todb-init action, which uses Go to install Goose to set up the database, and which runs in a Docker container. So download_go needs to be re-added to the todb-init action, too.

Copy link
Copy Markdown
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

Looks good so far. Next, for each workflow that uses a GitHub Action that you removed download_go from, get the Go version from the GO_VERSION file and install that Go version using the actions/setup-go action:

Go version should be be set in the workflow files by

  1. Checking the version in the GO_VERSION file:
    - name: go-version
    run: echo "::set-output name=value::$(cat GO_VERSION)"

    Note that id also needs to be set for this step (see #5991).
  2. Using the actions/setup-go action with that version.
    - uses: actions/setup-go@v2
    with:
    go-version: ${{ steps.go-version.outputs.value }} # The Go version to download (if necessary) and use.

@y377
Copy link
Copy Markdown

y377 commented Jul 17, 2021

Installing

Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of RAM, and 20 GB of disk space

hi,guys!
Can a cloud server with 1cpu and 2GB ram be installed?

@mitchell852 mitchell852 added automation related to automated testing/deployment/packaging etc. improvement The functionality exists but it could be improved in some way. tech debt rework due to choosing easy/limited solution labels Jul 19, 2021
- name: Save Alpine Docker image
run: .github/actions/save-alpine-tar/entrypoint.sh save ${{ env.ALPINE_VERSION }}

run: .github/actions/save-alpine-tar/entrypoint.sh save ${{ env.ALPINE_VERSION }} No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing terminating newline in text file

Copy link
Copy Markdown
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

The dashboard URLs still need to be reverted.
#6026 (comment)
#6026 (comment)

Copy link
Copy Markdown
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

@bueale bueale force-pushed the remove-duplicate-download-go-func branch from 75860ae to a6ad5a2 Compare July 22, 2021 20:34
abueno001 added 16 commits July 22, 2021 15:47
Functionality is handled within trafficcontrol/.github/workflows/go.vet.yml
Removing code assigning GOROOT, PATH
Some functions of download_go are still needed for git actions to succeed.
Updated functions to get Go Version and install Go
Changes to Install Go portion of workflow
Making changes suggested in comments.
Changing back to name=value as test failed. Need to look up proper way to set id.
Check Go Version has id set to go-version
Added steps to check go version and install
check-go-modules.yml, traffic-ops.yml, go.fmt.yml changed.
Moving go version check and install to bottom
Moving go install and version check to run after Go fmt
Moved location of Go install and version check.
@bueale bueale force-pushed the remove-duplicate-download-go-func branch from a6ad5a2 to dacb6e4 Compare July 22, 2021 20:48
Copy link
Copy Markdown
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

Looks good!

  • download_go functions and its remnants are removed from the non-Docker GHAs
  • Our GitHub Actions use actions/setup-go to download the Go version specified in the GO_VERSION file
  • The only GitHub Actions still running download_go are Docker actions that cannot use the actions/setup-go GHA
  • go-version id is set in the Go Vet workflow, which fixes #5991

The TP_Integration_tests fails, (stil fails after re-running) but because only 1 out of 490 specs failed, I do not think this PR caused the failure. The 1 failure:

2021-07-23T15:28:43.5354179Z 1) Traffic Portal - Origins - Operation Role can login
2021-07-23T15:28:43.5354891Z   Message:
2021-07-23T15:28:43.5360049Z     Failed: No element found using locator: By(css selector, *[id="loginUsername"])

We should open an Issue for the sporadic TP spec failure.

@zrhoffman
Copy link
Copy Markdown
Member

TP Tests passed this time after failing twice in a row.

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

Labels

automation related to automated testing/deployment/packaging etc. improvement The functionality exists but it could be improved in some way. tech debt rework due to choosing easy/limited solution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ATC GitHub Actions duplicate Go-downloading code Go Vet GitHub Action does not set Go version from GO_VERSION

6 participants