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

CI changes for new k8s images #560

Merged
merged 6 commits into from
Oct 26, 2023
Merged

Conversation

chinhtranvan
Copy link
Collaborator

@chinhtranvan chinhtranvan commented Oct 25, 2023

There is some image changes for this PR

  • remove the nokeys image from the pipeline and NO_KEYS env var usage
  • GH actions to stop using latest-master for vcluster tests. Instead use full or minimal, depending on the leg.

Copy link
Collaborator

@spilchen spilchen 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. I just had minor comments. It's too bad the e2e pipeline is slow today, we could have gotten a few runs in.

docker-vertica/Dockerfile Show resolved Hide resolved
docker-vertica/Dockerfile Outdated Show resolved Hide resolved
docker-vertica/Dockerfile Show resolved Hide resolved
.github/actions/setup-e2e/action.yaml Outdated Show resolved Hide resolved
.github/workflows/build-images.yml Outdated Show resolved Hide resolved
.github/workflows/e2e.yml Outdated Show resolved Hide resolved
.github/workflows/e2e.yml Outdated Show resolved Hide resolved
.github/workflows/e2e.yml Outdated Show resolved Hide resolved
.github/workflows/e2e.yml Show resolved Hide resolved
.github/workflows/e2e.yml Outdated Show resolved Hide resolved
@spilchen
Copy link
Collaborator

run some e2e legs against a released vertica-k8s image (vertica/vertica-k8s:23.3.0-0)

I never saw a change for this. If so, it's fine to leave off of this PR. I think we have enough here.

@chinhtranvan
Copy link
Collaborator Author

run some e2e legs against a released vertica-k8s image (vertica/vertica-k8s:23.3.0-0)

I never saw a change for this. If so, it's fine to leave off of this PR. I think we have enough here.

Yes, I will leave off of this PR

@chinhtranvan chinhtranvan force-pushed the chinh/ci-changes-for-new-k8s-images branch from a991f06 to b33c370 Compare October 26, 2023 04:47
@chinhtranvan
Copy link
Collaborator Author

chinhtranvan commented Oct 26, 2023

The e2e test looks weird. I'm trying to understand why there is a 'no such file or directory' error when trying to open full-vertica-image/full-vertica-image.tar. Do we need to wait for JenKins to trigger GitHub CI?

Comment on lines -129 to -133
- uses: actions/upload-artifact@v3
if: ${{ github.event_name == 'pull_request' }}
with:
name: full-vertica-image
path: full-vertica-image.tar
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is needed so that the full image is available for later stages.

@spilchen
Copy link
Collaborator

The e2e test looks weird. I'm trying to understand why there is a 'no such file or directory' error when trying to open full-vertica-image/full-vertica-image.tar. Do we need to wait for JenKins to trigger GitHub CI?

There is a missing actions/upload-artifact step. Something like this is needed:

    - uses: actions/upload-artifact@v3
      if: ${{ github.event_name == 'pull_request' }}
      with:
        name: full-vertica-image
        path: full-vertica-image.tar

@chinhtranvan
Copy link
Collaborator Author

chinhtranvan commented Oct 26, 2023

The e2e test looks weird. I'm trying to understand why there is a 'no such file or directory' error when trying to open full-vertica-image/full-vertica-image.tar. Do we need to wait for JenKins to trigger GitHub CI?

There is a missing actions/upload-artifact step. Something like this is needed:

    - uses: actions/upload-artifact@v3
      if: ${{ github.event_name == 'pull_request' }}
      with:
        name: full-vertica-image
        path: full-vertica-image.tar

Oh, I remember why we don't need the path full-vertica-image.tar—we encountered this problem before. For v2-vertica-image, we won't push the image because the RPM we use is too old. Therefore, there's no need to save it under full-vertica-image/full-vertica-image.tar and load it later. We just need to pull the image. Reference PR: https://github.com/vertica/vertica-kubernetes/pull/440/files

After skip loading the images, we passed all e2e vcluster tests, only have problems for AT tests, I see errors in AT tests
scripts/push-to-kind.sh: option requires an argument -- i
ERROR: unrecognized option: -?

@chinhtranvan chinhtranvan force-pushed the chinh/ci-changes-for-new-k8s-images branch from 7c02471 to 0e67ac8 Compare October 26, 2023 14:58
@chinhtranvan
Copy link
Collaborator Author

chinhtranvan commented Oct 26, 2023

The e2e test looks weird. I'm trying to understand why there is a 'no such file or directory' error when trying to open full-vertica-image/full-vertica-image.tar. Do we need to wait for JenKins to trigger GitHub CI?

There is a missing actions/upload-artifact step. Something like this is needed:

    - uses: actions/upload-artifact@v3
      if: ${{ github.event_name == 'pull_request' }}
      with:
        name: full-vertica-image
        path: full-vertica-image.tar

Oh, I remember why we don't need the path full-vertica-image.tar—we encountered this problem before. For v2-vertica-image, we won't push the image because the RPM we use is too old. Therefore, there's no need to save it under full-vertica-image/full-vertica-image.tar and load it later. We just need to pull the image. Reference PR: https://github.com/vertica/vertica-kubernetes/pull/440/files

After skip loading the images, we passed all e2e vcluster tests, only have problems for AT tests, I see errors in AT tests scripts/push-to-kind.sh: option requires an argument -- i ERROR: unrecognized option: -?

Update: just fix the problem, build-e2e.yml

The e2e test looks weird. I'm trying to understand why there is a 'no such file or directory' error when trying to open full-vertica-image/full-vertica-image.tar. Do we need to wait for JenKins to trigger GitHub CI?

There is a missing actions/upload-artifact step. Something like this is needed:

    - uses: actions/upload-artifact@v3
      if: ${{ github.event_name == 'pull_request' }}
      with:
        name: full-vertica-image
        path: full-vertica-image.tar

Oh, I remember why we don't need the path full-vertica-image.tar—we encountered this problem before. For v2-vertica-image, we won't push the image because the RPM we use is too old. Therefore, there's no need to save it under full-vertica-image/full-vertica-image.tar and load it later. We just need to pull the image. Reference PR: https://github.com/vertica/vertica-kubernetes/pull/440/files

After skip loading the images, we passed all e2e vcluster tests, only have problems for AT tests, I see errors in AT tests scripts/push-to-kind.sh: option requires an argument -- i ERROR: unrecognized option: -?

Just fix the problem: some previous changes in build-image.yml caused an empty Vertica Image. All comments have been addressed with successful e2e tests. After this CI is merged, I will go back to Jenkin to trigger CI.

@spilchen spilchen merged commit 5a2e16a into main Oct 26, 2023
24 checks passed
@spilchen spilchen deleted the chinh/ci-changes-for-new-k8s-images branch October 26, 2023 19:30
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