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

Set up minikube for k8s testing #15826

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

umohnani8
Copy link
Member

Install and set up minikube so that we can
create a k8s cluster for testing.

Signed-off-by: Urvashi Mohnani [email protected]

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 15, 2022
@umohnani8
Copy link
Member Author

@cevich PTAL

contrib/cirrus/runner.sh Outdated Show resolved Hide resolved
@cevich
Copy link
Member

cevich commented Sep 15, 2022

Looks like a good start to me. To speed up development, please feel free to do a temporary DO NOT MERGE commit that removes the rootless_system_test dependency. For other tasks (except build), you can bypass them by (temporarily) adding a skip: $CI == $CI item to them. That'll should get the minikube task running earlier so you can iterate faster 😁

@cevich
Copy link
Member

cevich commented Sep 15, 2022

Oh, and you'll want to change IMAGE_SUFFIX: "c5823947156488192" to IMAGE_SUFFIX: "c5167094042984448". The older images don't have the minikube rpm cached in them.

@cevich
Copy link
Member

cevich commented Sep 27, 2022

FYI: The new task is throwing Error: Unable to find a match: /var/cache/download/minikube-latest* because the IMAGE_SUFFIX in .cirrus.yml is wrong (see my comment above).

.cirrus.yml Outdated Show resolved Hide resolved
@umohnani8
Copy link
Member Author

@cevich are these failures related to my changes? Did I mess up something?

@umohnani8
Copy link
Member Author

@cevich any idea why the minikube test was cancelled?

@edsantiago
Copy link
Member

@umohnani8 the minikube test depends on rootless_system_test, which in turn depends on rootless_integration_test, which failed. The failure looks real, because many tests failed in the same way. I suspect a problem with the VM images.

@cevich
Copy link
Member

cevich commented Oct 3, 2022

Ahh this is the "os error 95" thing. This is caused by a bug in netavark (I think). There's a fix that should be in the stable repos by now.. Getting it into CI means building new VM Images. I've got an unrelated PR doing that now, I just need to flip some switches. Can you wait a day or two or you need something sooner?

@umohnani8
Copy link
Member Author

@cevich thanks, I can wait a day or so for it to be done.

@cevich
Copy link
Member

cevich commented Oct 3, 2022

Great, if that changes let me know and I'll do a separate build for this. Otherwise they can come from containers/automation_images#213 - if only the damn CI would pass (lol...it's my fault, I'm trying to add new tests).

@cevich
Copy link
Member

cevich commented Oct 4, 2022

okay @umohnani8 I think this should work. Give IMAGE_SUFFIX: "c4678746211876864" a try and let me know.

@TomSweeneyRedHat
Copy link
Member

LGTM
once the tests are hip. Hopefully @cevich 's thought cures all ills, at least in this PR.

test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Came here to agree with @cevich's assessments, found a few more concerns

test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
Copy link
Member

@edsantiago edsantiago 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 moving waaaaaaaaaaaaay too fast for slow old me. I will step back until things calm down (like, tomorrow morning).

test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
contrib/cirrus/runner.sh Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Final set of comments on this iteration. All of them are nits: in broad sense, looks like good testing. Thanks for your tolerance of my nitpicking!

test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Outdated Show resolved Hide resolved
test/minikube/001-kube.bats Show resolved Hide resolved
@umohnani8 umohnani8 force-pushed the minikube branch 2 times, most recently from b458505 to 1638adb Compare October 6, 2022 17:42
@edsantiago
Copy link
Member

Reviewing again. The new helpers.bash needs a trailing newline to pass validation. Since you need to resubmit anyway, would you consider calling basic_setup from your setup? It will prefetch $IMAGE and do some cleanup that might give you an extra edge in running tests after one test fails.

@umohnani8
Copy link
Member Author

Thanks for all the reviews @edsantiago and @cevich!

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 6, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, umohnani8

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [edsantiago,umohnani8]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@edsantiago
Copy link
Member

New tests failing:

[+0004s] not ok 1 minikube - check cluster is up
         # (from function `setup' in test file test/minikube/[helpers.bash, line 8](https://github.com/containers/podman/blob/4a04b156eeccf30fcfedf4175f09edbc1c407728/test/system/helpers.bash#L8))
         #   `minikube start' failed with status 57
         # * minikube v1.27.0 on Fedora 36 (kvm/amd64)
         # ! Kubernetes 1.25.0 has a known issue with resolv.conf. minikube is using a workaround that should work for most use cases.
         # ! For more information, see: https://github.com/kubernetes/kubernetes/issues/112135
         # * Using the podman driver based on user configuration
         # * The "podman" driver should not be used with root privileges. If you wish to continue as root, use --force.
         # * If you are running minikube within a VM, consider using --driver=none:
         # *   https://minikube.sigs.k8s.io/docs/reference/drivers/none/
         #
         # X Exiting due to DRV_AS_ROOT: The "podman" driver should not be used with root privileges.

Not something I can help with, sorry. Or at least, easily help with. Am posting in case it's something trivial for you to identify and fix. If it isn't, lmk and I'll spin up a VM.

@edsantiago
Copy link
Member

The sys failure is a flake, I've filed #16075 for it but there's no point in rerunning it. Anyhow, don't sweat that one.

@umohnani8
Copy link
Member Author

@cevich can minikube be run rootless? root privilege are not required for it when using podman

Install and set up minikube so that we can
create a k8s cluster for testing.

Signed-off-by: Urvashi Mohnani <[email protected]>
@umohnani8
Copy link
Member Author

All tests are finally green! @cevich @edsantiago can I please get a final lgtm to merge.

@edsantiago
Copy link
Member

git range-diff confirms that the only changes since my last review are rootless and a rebase... so I haven't done a thorough re-review. Feel free to release hold, or wait for another set of eyes.

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2022
@rhatdan
Copy link
Member

rhatdan commented Oct 19, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2022
@openshift-merge-robot openshift-merge-robot merged commit f6053ce into containers:main Oct 19, 2022
@cevich
Copy link
Member

cevich commented Oct 19, 2022

Nice work on this @umohnani8 Thanks for seeing it through.

@umohnani8 umohnani8 deleted the minikube branch February 23, 2023 18:44
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 8, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants