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

fix(ci): build images before running sanity test #129

Merged
merged 7 commits into from
Dec 13, 2021

Conversation

niladrih
Copy link
Member

@niladrih niladrih commented Dec 9, 2021

Signed-off-by: Niladri Halder [email protected]

Why is this PR required? What issue does it fix?:
The GitHub Actions CI runs make sanity-test against existing docker hub images. The images with the changes are not built correctly before running the sanity test. The sanity test does not produce results against the latest changes.

Additionally, the apt install for the nfs-common prerequisite will require interactive Y/N command if the nfs-common prerequisite is absent, and the install needs to go through. Added the -y flag to make the Y option non-interactive.

What this PR does?:
Build workflow: Both the provisioner-nfs and nfs-server-alpine image builds are added before the test run.
Pull Request workflow: The build for the nfs-server-alpine image is now added.

The Dockerfile file in buildscripts/provisioner-nfs/ directory now uses the same base image as provisioner-nfs.Dockerfile.
Changed the CMD directive to ENTRYPOINT directive in 'Dockerfile' file.

Go setup in GitHub actions is added to provisioner-nfs workflow.

Updated minikube action to latest version -- 2.4.2
Minikube used is now v1.24.0
Kubernetes version is now 1.20.13
Added flag to disable minikube storage-provisioner addon (nfs_claim_released_pv test, nfs_sc_delayed_binding_test.go, nfs_server_param_test.go now use openebs-hostpath BackendStorageClass instead of the default one)

Build and Pull Request workflow runs will be avoided for pushes to docs/ directory

The sanity-test now runs in parallel with the multi-arch build tests for the provisioner-nfs and nfs-server-alpine image.

Does this PR require any upgrade changes?:
No.

If the changes in this PR are manually verified, list down the scenarios covered::
Not manually verified.

Any additional information for your reviewer? :
The PR #125 depends on this.

@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2021

Codecov Report

Merging #129 (e259953) into develop (59d00f6) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #129   +/-   ##
========================================
  Coverage    49.79%   49.79%           
========================================
  Files           38       38           
  Lines         2705     2705           
========================================
  Hits          1347     1347           
  Misses        1255     1255           
  Partials       103      103           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 347ab2f...e259953. Read the comment docs.

niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 10, 2021
niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 10, 2021
niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 10, 2021
niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 10, 2021
niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 10, 2021
niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 10, 2021
…anity-test job item higher up the list for readability

Signed-off-by: Niladri Halder <[email protected]>
Signed-off-by: Niladri Halder <[email protected]>
@@ -12,7 +12,7 @@
#See the License for the specific language governing permissions and
#limitations under the License.
#
FROM alpine:3.11.5
FROM alpine:3.12
Copy link
Contributor

Choose a reason for hiding this comment

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

Info: Changes in this file are done to sync with buildx docker file.

Copy link
Member Author

Choose a reason for hiding this comment

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

✔️

Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

Changes are good

Copy link
Contributor

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

LGTM

@mittachaitu mittachaitu merged commit 3ce5a7d into openebs-archive:develop Dec 13, 2021
niladrih added a commit to niladrih/dynamic-nfs-provisioner that referenced this pull request Dec 13, 2021
# to ignore builds on release
if: ${{ (github.event.ref_type != 'tag') }}
runs-on: ubuntu-latest
needs: ['lint', 'unit-test']
Copy link
Contributor

Choose a reason for hiding this comment

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

@niladrih Running sanity test parallel is causing test to get timeout(May be due to less resources)

Copy link
Member Author

Choose a reason for hiding this comment

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

One of the tests failed a due to 404 error.

2 other sanity-tests running in #272 and #273 passed.

Likely cause is test flakiness.

@niladrih niladrih deleted the ci branch January 3, 2022 15:01
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.

3 participants