Moving cluster logic into it's own object #649

chasain · 2021-04-16T22:54:32Z

This PR makes use of bazel 4.0 custom flags to define the target cluster it moves much of the logic from k8s_object regarding the target cluster into it's own rule (cluster) and makes use of an aspect to pull in information. Example usage would be:

clusters(
    name = "clusters",
    clusters = [
        ":minikube",
    ],
)

cluster(
    name = "minikube",
    cluster = "minikube",
    image_chroot = "localhost:5000",
    substitutions = {
        "%{host_name}": "localhost",
        "%{http_protocol}": "http",
    },
)

k8s_default needs to have a clusters = //path/to/clusters:label added to work then the users bazelrc needs to be edited to include :

build --flag_alias=cluster=@io_bazel_rules_k8s//k8s:cluster_flag

Then the user can make use of the flag by calling

bazel run //path/to/k8s:object --cluster=minikube

k8s-ci-robot · 2021-04-16T22:54:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chasain
To complete the pull request process, please assign chizhg after the PR has been reviewed.
You can assign the PR to them by writing /assign @chizhg in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

k8s-ci-robot · 2021-04-16T22:54:40Z

Hi @chasain. Thanks for your PR.

I'm waiting for a bazelbuild member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

fejta · 2021-04-21T22:25:15Z

Hi @chasain any thoughts on how you might unit test this new behavior?

UebelAndre · 2021-04-22T14:47:43Z

There also is no way to e2e test with Bazel 4.0 (#639 (comment)). Not sure if that'd impact anything here.

chasain · 2021-04-22T15:04:45Z

There also is no way to e2e test with Bazel 4.0 (#639 (comment)). Not sure if that'd impact anything here.

Looks like this is the tracker for that: GoogleCloudPlatform/container-definitions#12037

fejta · 2021-04-30T21:44:19Z

(does reopening cause buildkite to retry? looks like no)

fejta · 2021-04-30T21:56:16Z

/ok-to-test

fejta · 2021-05-01T00:54:09Z

The e2e tests provide a good signal again, will you push a commit which addresses the pull-rules-k8s-e2e errors? Thanks!

chasain · 2021-05-10T19:07:38Z

Hey, was dealing with some personal stuff last week, I'll take a look at it this week.

fejta · 2021-05-10T20:07:55Z

No problem, thanks for you patience with the e2e errors and for contributing to this repo! Will review it again once there's a new commit up.

fejta · 2021-05-10T20:58:09Z

.bazelversion

@@ -1 +1 @@
-3.5.0
+4.0.0


We don't have 4.0.0 images yet on account of the fact that launcher.gcr.io/google/bazel hasn't published any images since 3.5.0

We'll either need to wait for that to get fixed (there's an open issue somewhere to publish them (which no one seems to be addressing)

Or else change the Dockerfile to install both targeted versions of bazel using some other mechanism (aka building both from source)

rules_k8s/images/gcloud-bazel/Dockerfile

Line 19 in 32df719

FROM launcher.gcr.io/google/bazel:${OLD_VERSION} as old

bazelbuild/continuous-integration#1060

Yeah, I added that to be able to test locally since the flags are 4.0 only, but it actually seemed to work ok in the buildkite job...

The buildkite job just does bazelisk build //... which is an important signal.

Actually using the outputs to talk to kubectl is handled by the pull-rules-k8s-e2e job which has an image with both 3.5.0 and 3.0.0, which can be chosen using .bazelversion

Do you need to build and push a new image when I change that Dockerfile? I added what the job recommended but it looks like it's still the same image.

Bazel binaries for all official releases can be downloaded from here: https://github.com/bazelbuild/bazel/releases You can download the required version directly using this command: (cd "/usr/local/lib/bazel/bin" && curl -fLO https://releases.bazel.build/4.0.0/release/bazel-4.0.0-linux-x86_64 && chmod +x bazel-4.0.0-linux-x86_64)

chasain · 2021-05-11T16:01:48Z

~~Not sure how to update minimum supported version to 4.0~~
Figured it out

k8s-ci-robot · 2021-05-11T19:28:54Z

@chasain: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-rules-k8s-e2e	`40e4e2e`	link	`/test pull-rules-k8s-e2e`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fejta · 2021-05-11T19:47:09Z

If your PR requires bazel 4.0 I'd suggest waiting until next week. I should be able to push a new image then (it isn't an automatic process)

…

On Tue, May 11, 2021, 12:29 Kubernetes Prow Robot ***@***.***> wrote: @chasain <https://github.com/chasain>: The following test *failed*, say /retest to rerun all failed tests: Test name Commit Details Rerun command pull-rules-k8s-e2e 40e4e2e <40e4e2e> link <https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/bazelbuild_rules_k8s/649/pull-rules-k8s-e2e/1392200017061613568> /test pull-rules-k8s-e2e Full PR test history <https://prow.k8s.io/pr-history?org=bazelbuild&repo=rules_k8s&pr=649>. Your PR dashboard <https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Achasain>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/bazelbuild/rules_k8s/issues?q=is:issue+is:open> when you hit one in your PR. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#649 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFSNP426DUTQEIAP4WDALTNGAQ3ANCNFSM43CLV4VQ> .

chasain · 2021-05-24T17:58:38Z

Any updates on 4.0?

fejta · 2021-07-15T07:27:49Z

Has most of the work, but java tests aren't passing for mysterious reasons. Expect to solve this a week from now: #664

k8s/object.bzl

+ kind = kwargs.get("kind"),
+ clusters = kwargs.get("clusters"),
+ user = kwargs.get("user"),
+ namespace = kwargs.get("namespace"),


fejta · 2022-06-10T21:33:26Z

Would you like to try and rebase this PR or close it?

chasain · 2022-06-10T22:11:47Z

I ended up moving to skaffold to manage this process in a much cleaner way, feel free to close this.

k8s-ci-robot requested review from chases2 and fejta April 16, 2021 22:54

k8s-ci-robot added needs-ok-to-test size/L labels Apr 16, 2021

fejta closed this Apr 30, 2021

fejta reopened this Apr 30, 2021

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Apr 30, 2021

Charley Saint added 2 commits May 10, 2021 15:51

Moving cluster logic into it's own object

9c232a7

Fixing some issue blocking e2e testing

af7be5c

chasain force-pushed the master branch from b4d3143 to af7be5c Compare May 10, 2021 20:53

fejta reviewed May 10, 2021

View reviewed changes

Charley Saint added 2 commits May 11, 2021 09:54

Fixing linting errors and removing namespace flag

bbeb928

Testing new bazel image and fixing linting errors

3d34166

chasain force-pushed the master branch from 43e2062 to 8711266 Compare May 11, 2021 19:19

Manually installing the correct bazel version

0331d3d

chasain force-pushed the master branch from 8711266 to 0331d3d Compare May 11, 2021 19:24

Switching bazel image in WORKSPACE

40e4e2e

moriarty reviewed Aug 5, 2021

View reviewed changes

k8s/object.bzl

kind = kwargs.get("kind"),

clusters = kwargs.get("clusters"),

user = kwargs.get("user"),

namespace = kwargs.get("namespace"),

This comment was marked as off-topic.

Sign in to view

This comment was marked as resolved.

Sign in to view

chasain closed this Jun 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving cluster logic into it's own object #649

Moving cluster logic into it's own object #649

chasain commented Apr 16, 2021

k8s-ci-robot commented Apr 16, 2021

k8s-ci-robot commented Apr 16, 2021

fejta commented Apr 21, 2021

UebelAndre commented Apr 22, 2021

chasain commented Apr 22, 2021

fejta commented Apr 30, 2021

fejta commented Apr 30, 2021

fejta commented May 1, 2021

chasain commented May 10, 2021

fejta commented May 10, 2021

fejta May 10, 2021

fejta May 10, 2021

chasain May 10, 2021

fejta May 10, 2021

chasain May 11, 2021

chasain commented May 11, 2021 •

edited

Loading

k8s-ci-robot commented May 11, 2021

fejta commented May 11, 2021 via email

chasain commented May 24, 2021

fejta commented Jul 15, 2021

This comment was marked as off-topic.

This comment was marked as resolved.

fejta commented Jun 10, 2022

chasain commented Jun 10, 2022

		@@ -1 +1 @@
		3.5.0
		4.0.0

Moving cluster logic into it's own object #649

Moving cluster logic into it's own object #649

Conversation

chasain commented Apr 16, 2021

k8s-ci-robot commented Apr 16, 2021

k8s-ci-robot commented Apr 16, 2021

fejta commented Apr 21, 2021

UebelAndre commented Apr 22, 2021

chasain commented Apr 22, 2021

fejta commented Apr 30, 2021

fejta commented Apr 30, 2021

fejta commented May 1, 2021

chasain commented May 10, 2021

fejta commented May 10, 2021

fejta May 10, 2021

Choose a reason for hiding this comment

fejta May 10, 2021

Choose a reason for hiding this comment

chasain May 10, 2021

Choose a reason for hiding this comment

fejta May 10, 2021

Choose a reason for hiding this comment

chasain May 11, 2021

Choose a reason for hiding this comment

chasain commented May 11, 2021 • edited Loading

k8s-ci-robot commented May 11, 2021

fejta commented May 11, 2021 via email

chasain commented May 24, 2021

fejta commented Jul 15, 2021

This comment was marked as off-topic.

This comment was marked as resolved.

fejta commented Jun 10, 2022

chasain commented Jun 10, 2022

chasain commented May 11, 2021 •

edited

Loading