Skip to content

ci: remove kubebuilder dependency#279

Merged
sozercan merged 2 commits into
open-policy-agent:masterfrom
sozercan:ci-remove-kubebuilder
Jan 21, 2023
Merged

ci: remove kubebuilder dependency#279
sozercan merged 2 commits into
open-policy-agent:masterfrom
sozercan:ci-remove-kubebuilder

Conversation

@sozercan
Copy link
Copy Markdown
Member

Signed-off-by: Sertac Ozercan sozercan@gmail.com

@sozercan sozercan force-pushed the ci-remove-kubebuilder branch 2 times, most recently from c26d392 to 376db8d Compare January 19, 2023 06:45
@sozercan
Copy link
Copy Markdown
Member Author

GK test should succeed after open-policy-agent/gatekeeper#2524 is merged

@sozercan sozercan requested a review from maxsmythe January 19, 2023 18:45
name: "Unit test"
runs-on: ubuntu-latest
timeout-minutes: 5
timeout-minutes: 10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hm, did you notice an increase in timeouts?

Copy link
Copy Markdown
Member Author

@sozercan sozercan Jan 19, 2023

Choose a reason for hiding this comment

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

yes, i added -race -bench to align with GK which increased the timeouts (mostly due to benchmarks)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the reason we added is to make sure to run benchmarks often so we don't break them

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
@sozercan sozercan force-pushed the ci-remove-kubebuilder branch from 376db8d to bed205d Compare January 19, 2023 22:22
Comment thread constraint/Makefile Outdated
--build-arg KUSTOMIZE_VERSION=$(KUSTOMIZE_VERSION)

## Location to install dependencies to
LOCALBIN ?= $(shell pwd)/bin
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If a user runs this in their source code directory, this will create a /bin folder. Do we want to put this in an easy-to-clean subdir (.output or similar)? Is the /bin directory part of .gitignore?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like bin would be ignored, though I'd personally consider putting this in a .output directory to make it easy to wipe everything and start from zero.

Copy link
Copy Markdown
Member Author

@sozercan sozercan Jan 20, 2023

Choose a reason for hiding this comment

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

@maxsmythe bin is the default for kubebuilder v3 and it's in gitignore. looks like we are using .tmp already (for linter) so i changed to .tmp/bin

Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread constraint/Makefile Outdated
--build-arg KUSTOMIZE_VERSION=$(KUSTOMIZE_VERSION)

## Location to install dependencies to
LOCALBIN ?= $(shell pwd)/bin
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like bin would be ignored, though I'd personally consider putting this in a .output directory to make it easy to wipe everything and start from zero.

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
@sozercan sozercan requested a review from maxsmythe January 20, 2023 18:56
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Base: 49.30% // Head: 49.30% // No change to project coverage 👍

Coverage data is based on head (9fc37f8) compared to base (b745745).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #279   +/-   ##
=======================================
  Coverage   49.30%   49.30%           
=======================================
  Files          69       69           
  Lines        4468     4468           
=======================================
  Hits         2203     2203           
  Misses       2021     2021           
  Partials      244      244           
Flag Coverage Δ
unittests 49.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

Sorry, I thought I LGTMd this after verifying bin was git-ignored (meant that to be a nit)

@maxsmythe
Copy link
Copy Markdown
Contributor

It looks like the tests are now failing. If moving the binaries makes it harder to start the control plane, let's just use the vanilla location.

@sozercan sozercan merged commit 5e694bd into open-policy-agent:master Jan 21, 2023
@sozercan sozercan deleted the ci-remove-kubebuilder branch January 21, 2023 04:28
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.

4 participants