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

Refactor test and release workflows #234

Merged
merged 4 commits into from
Mar 22, 2022
Merged

Conversation

detiber
Copy link
Member

@detiber detiber commented Mar 18, 2022

  • Add support for multiple registries
  • conditionalize registry logins and image pushes
  • add dockerignore
  • Cleanup image creation
  • bump golangci-lint
  • use golangci-lint for vet, use golangci github action for lint
  • Add caching for ci/release workflows
  • breakout lint, test, image as separate jobs
  • cleanup makefile a bit more
  • Add dependabot config
  • Skip ci workflow on dependabot branches
  • Update default image used by helm chart and deployment.yaml

Signed-off-by: Jason DeTiberus detiber@users.noreply.github.com

@detiber
Copy link
Member Author

detiber commented Mar 18, 2022

Examples of these workflows running can be found here: https://github.com/detiber/packet-ccm/actions
Example releases can be found here: https://github.com/detiber/packet-ccm/releases

Note: only a few of the example releases have meaningful notes because there were no PRs with the exception of the dependabot PRs...

.dockerignore Show resolved Hide resolved
.github/dependabot.yml Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
.github/workflows/ci.yaml Show resolved Hide resolved
.golangci.yml Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
fmt: ## Format all source code files
$(BUILD_CMD) gofmt -w -s ${GO_FILES}
fmt: golangci-lint ## Format all source code files
@$(BUILD_CMD) $(LINTER) run --fix ./
Copy link
Member Author

Choose a reason for hiding this comment

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

standardize on golangci-lint, this also attempts to auto fix any issues similar to the old fmt target.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@detiber
Copy link
Member Author

detiber commented Mar 18, 2022

Looks like I need to bump up the golangci-lint timeout a bit more.

It also looks like I need to not attempt to push the built image when it's a PR (which makes sense given PRs don't have access to the repo secrets).

@deitch
Copy link
Contributor

deitch commented Mar 21, 2022

This is quite the cleanup! Thanks for tackling it. I only had a few questions, mostly marked inline.

@deitch
Copy link
Contributor

deitch commented Mar 21, 2022

This is looking pretty impressive. It needs a rebase, and I think we should leave the pushed images solely for master and semver tags for now. We always can open a PR to focus just on that discussion in the future.

- Add support for multiple registries
- conditionalize registry logins and image pushes
- add dockerignore
- Cleanup image creation
- bump golangci-lint
- use golangci-lint for vet, use golangci github action for lint
- Add caching for ci/release workflows
- breakout lint, test, image as separate jobs
- cleanup makefile a bit more
- Add dependabot config
- Skip ci workflow on dependabot branches
- Update default image used by helm chart and deployment.yaml

Signed-off-by: Jason DeTiberus <detiber@users.noreply.github.com>
Signed-off-by: Jason DeTiberus <detiber@users.noreply.github.com>
Signed-off-by: Jason DeTiberus <detiber@users.noreply.github.com>
Signed-off-by: Jason DeTiberus <detiber@users.noreply.github.com>
@detiber
Copy link
Member Author

detiber commented Mar 21, 2022

This is looking pretty impressive. It needs a rebase, and I think we should leave the pushed images solely for master and semver tags for now. We always can open a PR to focus just on that discussion in the future.

Updated for at least the push/pr based job, it will now only build the images when it's a push and when branch is 'master'.

For the tag-based release job, right now it only enforces that the tag start with a v, not that the tag specifically relates to a semantic version, but this was the previous behavior, so it's not a regression in behavior.

@deitch
Copy link
Contributor

deitch commented Mar 22, 2022

I changed the settings to require your new job names instead of the old ones. Looks good now.

@deitch deitch merged commit 04ffe40 into kubernetes-sigs:master Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants