use release artifacts for e2e tests#152
use release artifacts for e2e tests#152ajaysundark wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
k8s-ci-robot
added
the
kind/feature
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ajaysundark The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
✅ Deploy Preview for node-readiness-controller ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| cmd := exec.Command("make", "docker-build", fmt.Sprintf("IMG_PREFIX=%s", imagePrefix), fmt.Sprintf("IMG_TAG=%s", imageTag)) | ||
| _, err := utils.Run(cmd) | ||
| ExpectWithOffset(1, err).NotTo(HaveOccurred(), "Failed to build the manager(Operator) image") | ||
| if !skipBuild { |
There was a problem hiding this comment.
Any particular reason why we want to skip the build? am assuming this is for the case where image is already built and loaded in cluster, mainly when running e2e in local env
There was a problem hiding this comment.
Hi Avinesh, thanks for taking a look. The goal for this PR was to be able to use releasable artifacts in our end to end test flows.
While handling last release, I felt that the verification part -- https://github.com/kubernetes-sigs/node-readiness-controller/blob/main/RELEASE.md#4-final-verification-and-github-release could be automated by generating the manifests and run tests with these manifests before uploading them to github.
These code-changes are a enabler to plumb the manifests into the test framework.
| Expect(err).NotTo(HaveOccurred(), "Failed to install CRDs") | ||
| if useReleaseArtifacts { | ||
| By("installing CRDs from dist/crds.yaml") | ||
| cmd = exec.Command("kubectl", "apply", "-f", "dist/crds.yaml") |
There was a problem hiding this comment.
This would mean that dist/crds.yaml already exists. Should we add a check here or some logic to ensure it is always there when running e2e-tests?
There was a problem hiding this comment.
As I said above, the intention for these changes are release automation, if dependencies exist it may be better for the tests to fail for us to take notice than to check and gracefully fail. wdyt?
| _, err = utils.Run(cmd) | ||
| Expect(err).NotTo(HaveOccurred(), "Failed to deploy the controller-manager") | ||
| By("deploying the controller-manager from dist/install.yaml") | ||
| cmd = exec.Command("kubectl", "apply", "-f", "dist/install.yaml") |
There was a problem hiding this comment.
same, this needs install.yaml
ajaysundark
requested review from
AvineshTripathi
and removed request for
dchen1107 and
mrunalp
|
@AvineshTripathi do you have more thoughts on this? |
3 participants
Description
Enable testing with released manifests directly to help with qualification
Related Issue
xref: #151
Type of Change
/kind feature
Testing
Ran e2e tests with env flags
Checklist
make testpassesmake lintpassesDoes this PR introduce a user-facing change?
Doc #151