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

Make github actions more feature complete #1418

Merged

Conversation

TomHellier
Copy link
Contributor

@TomHellier TomHellier commented Dec 3, 2021

@liyinan926 Please before merging this change, could you look at adding the gcr.io registry secrets into github secrets? Please see this section of the github workflows.
https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/blob/master/.github/workflows/release.yaml#L31-L39

This change renames the github workflows to be clearer about their purpose, and adds a set of tests which
aim to force developers to increment the appVersion if they have changed anything with the spark-operator docker container
or the chart version if they have updated the chart.

These changes should hopefully enforce that docker images & helm charts are released when changes are made to them, and remove manual steps required by @liyinan926

Main Changes:

  • Added multiple github workflows to test
    • build-api-docs
      • fails if the apis change
      • generates the documentation, and fails the committed api-docs.md is different
    • build-sparkctl
      • ensures sparkctl builds sucessfully
    • build-spark-operator
      • ensures the spark-operator container can build
      • passes unit tests
      • no issues with govet
      • no issues with gofmt
      • Requires an update to the appVersion of the helm chart if changes to the docker container are in the PR
    • build-helm-chart
      • Builds the helm chart
      • produces the documentation
      • lints the chart
      • asserts that the chart version changes if any helm changes have occurred
      • installs the helm chart into a minikube cluster
      • checks for crd drift between chart and operator
    • integration-test

@TomHellier TomHellier force-pushed the test-build-docker-in-github-action branch from fef1ddf to 550e4c7 Compare December 3, 2021 12:59
@TomHellier TomHellier marked this pull request as ready for review December 3, 2021 13:02
@TomHellier TomHellier force-pushed the test-build-docker-in-github-action branch 11 times, most recently from 59c7a2d to 5fdc83b Compare December 3, 2021 15:33
@TomHellier TomHellier changed the title Test build docker in GitHub action Make github actions more feature complete Dec 3, 2021
This change renames the github workflows to be clearer about their purpose, and adds a set of tests which
aim to force developers to increment the appVersion if they have changed anything with the spark-operator docker container
or the chart version if they have updated the chart.

Signed-off-by: Tom Hellier <[email protected]>
@TomHellier TomHellier force-pushed the test-build-docker-in-github-action branch from 5fdc83b to 92c4cb8 Compare December 3, 2021 16:08
Copy link
Collaborator

@liyinan926 liyinan926 left a comment

Choose a reason for hiding this comment

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

This is awesome! Thanks for contributing!

@liyinan926 liyinan926 merged commit ba16242 into kubeflow:master Dec 3, 2021
@TomHellier TomHellier deleted the test-build-docker-in-github-action branch December 4, 2021 01:34
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
This change renames the github workflows to be clearer about their purpose, and adds a set of tests which
aim to force developers to increment the appVersion if they have changed anything with the spark-operator docker container
or the chart version if they have updated the chart.

Signed-off-by: Tom Hellier <[email protected]>
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.

2 participants