Initial implementation #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In particular, eirinix has moved from github.com/SUSE to code.cloudfoundry.org.
This pulls in newer logrus, which fixes issues with the case of the import path. Unfortunately, they hadn't make actual releases in forever, so we have to pick something off `master`; in this case, the version vendored by docker 19.03.12.
Since eirinix.Manager uses zap anyway, use it for main as well to ensure any logging we do will be in the same format.
This makes us always rebuild the binary when building the image, all in docker. This ensures that we have better control over the inputs for that build. The base image is still require for running, as (at least for now) the resulting binary depends on the dynamic linker. Also includes support to vendor any dependencies if we have any `replace` lines in `go.mod`; this is useful if we're using a locally edited version of those dependencies (the most likely of which is eirinix).
This is useful when developing to ensure the expected binary is in use.
This adds a make target to build the helm chart. Also fixes up the chart to actually work, including additional role binding (for the webhook namespace), etc.
This builds the whole thing, publishing docker images and helm charts (the latter of which goes in GitHub Pages).
While running on every push was good for development, we shouldn't do so for code that we hope will get merged.
viovanov
suggested changes
Aug 22, 2020
There was a problem hiding this comment.
Can you please make this a go binary + dockerfile, and move the helm part into kubecf.
We shouldn't change the scope of cloudfoundry-incubator/kubecf#1179
Otherwise 👍
viovanov
reviewed
Aug 22, 2020
viovanov
suggested changes
Aug 22, 2020
mook-as
commented
Aug 31, 2020
- Use kubecf-tools to determine the version, instead of `git describe` (directly, anyway) - Drop helm chart handling; assume the user will handle that in some other way (i.e. supply their own Kubernetes bits, which is the case for KubeCF) SUSE#1 (review)
3 tasks
7 tasks
viovanov
suggested changes
Sep 5, 2020
mook-as
commented
Sep 8, 2020
This is completely unused now.
If we turn off CGO for the binary (which doesn't appear to be needed), we can get a static binary that then doesn't need the dynamic linker (and therefore can bse based on an empty image). Note that we _do_ expect a working /tmp, so we will need to create that in the iamge first.
The workflow only builds the docker image; rename the workflow file to match that.
mook-as
added a commit
to mook-as/kubecf-tools
that referenced
this pull request
Sep 9, 2020
This was spawned out of SUSE/eirini-dns-aliases#1 to have a script that builds an arbitary go binary, controlled by a YAML file. Requested in SUSE/eirini-dns-aliases#1 (comment) This also makes the versioning script work better as a library (by not always parsing the command line options, only when it's run as the top level script).
mook-as
added a commit
to mook-as/kubecf-tools
that referenced
this pull request
Sep 9, 2020
This was spawned out of SUSE/eirini-dns-aliases#1 to have a script that builds an arbitrary go binary, controlled by a YAML file. Requested in SUSE/eirini-dns-aliases#1 (comment) This also makes the versioning script work better as a library (by not always parsing the command line options, only when it's run as the top level script).
mook-as
added a commit
to mook-as/kubecf-tools
that referenced
this pull request
Sep 9, 2020
This was spawned out of SUSE/eirini-dns-aliases#1 to have a script that builds an arbitrary go binary, controlled by a YAML file. Requested in SUSE/eirini-dns-aliases#1 (comment) This also makes the versioning script work better as a library (by not always parsing the command line options, only when it's run as the top level script).
This was referenced Sep 9, 2020
|
@mook-as I think you may need to adjust this PR to use the new kubecf-apps-dns. It was introduced by cloudfoundry-incubator/kubecf#1313. |
mook-as
force-pushed
the
f0rmiga/initial-impl
branch
from
699d3a6
to
87814da
Compare
viovanov
previously approved these changes
Sep 11, 2020
Use the GitHub container registry instead for image destination.
viovanov
approved these changes
Sep 11, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This expands upon f0rmiga/initial-impl to work; this has been tested on a KubeCF deployment.
Notable changes:
Note that I don't have write access to the repository, hence not just pushing on top of the branch. I also can't seem to request a review…