-
Notifications
You must be signed in to change notification settings - Fork 379
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
Add scaffolding to stand up Trillian on k8s. #2754
base: master
Are you sure you want to change the base?
Conversation
/gcbrun |
@roger2hk Thanks for the gcbrun. trillian-pr-tests are failing but I can't see why, can somebody share what the problem is so I can try to fix it :) |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oups, I did not intend to approve this PR just yet, still needs review. Please ignore it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few nits from a first glance. Do you want to rebase this onto HEAD, and meanwhile this week I'll try setting up an environment so I can follow the instructions here to really make sure it makes sense to me :-)
(apologies for late response, life has been busy the last 3 weeks!)
Thanks for the comments! Sorry for tardy response, was out on PTO, will pick this back up this week! @mhutchinson totes understood about life sometimes getting very busy :) |
/gcbrun |
``` | ||
3. Then create a tree in the Trillian: | ||
```shell | ||
ko apply -BRf ./examples/deployment/kubernetes/createtree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to add an issue (assigned to me) and TODO here to swap this over to use a pre-built image so that ko isn't needed.
/gcbrun |
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, modulo a few comments.
- name: Setup mirror | ||
uses: chainguard-dev/actions/setup-mirror@main | ||
with: | ||
mirror: mirror.gcr.io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this mirror?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some cases esp. in test environments this can reduce flakiness by not hitting rate limits on pulling containers. So by default we set this up before having to add it later :)
examples/deployment/kubernetes/scaffolding/log-server/placeholder.go
Outdated
Show resolved
Hide resolved
/gcbrun |
This is failing running the lint checks:
|
golangci/golangci-lint#1920 may be relevant. Holler if you can't work this out and I'll take a deeper look. |
/gcbrun |
This is still causing some problem with the linter. Fortunately it can be reproduced locally:
|
I think this is because client-go is min version go1.18: https://github.com/kubernetes/client-go/blob/master/go.mod golangci/golangci-lint#1920 (comment) edit: but then again we're using v0.23.8, which is go1.16: https://github.com/kubernetes/client-go/blob/v0.23.8/go.mod |
Hah, I've just spent ages messing about with looking into the linters and go versions. The answer appears to be as simple as running go build to find the root cause:
|
Root cause is probably kubernetes/client-go#1084. @vaikas can I leave this with you? |
Thank you so much @mhutchinson for digging into this. I'll keep looking at it, oh what joy! 🤣 |
Ok, added a replace to go.mod and now things are building:
Let's try this now :) |
/gcbrun |
Error is
Looks like a similar issue to the last one but with a different module. Oddly though, I can't reproduce this one locally with go build or golangci-lint. Will have a quick poke at this, but due to RL considerations I may need to leave this for @AlCutter to take over next week. |
@vaikas if you want to revisit this PR, the cloudbuild results are now viewable so it would be easier to debug. |
oooooh, yes, thank you for the pointer, I'll try to carve some time in the
next couple of days :)
…On Mon, Dec 12, 2022 at 10:30 PM Juan Antonio Osorio < ***@***.***> wrote:
@vaikas <https://github.com/vaikas> if you want to revisit this PR, the
cloudbuild results are now viewable so it would be easier to debug.
—
Reply to this email directly, view it on GitHub
<#2754 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACWB45GGBHAOLK7P5B3TB6DWNAJX3ANCNFSM5YCUENJA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I'm back! Getting this PR in would be a great accomplishment for my return if you're up for it @vaikas :-) |
Welcome back, happy new year!!!! Yes, I'll make time for it, this time I
really mean it.
…On Mon, Jan 9, 2023 at 2:04 PM Martin Hutchinson ***@***.***> wrote:
I'm back! Getting this PR in would be a great accomplishment for my return
if you're up for it @vaikas <https://github.com/vaikas> :-)
—
Reply to this email directly, view it on GitHub
<#2754 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACWB45CMHJYWF46YALXJZCTWRP5EBANCNFSM5YCUENJA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
b269b37
to
c8225b6
Compare
@mhutchinson would you mind hitting the magic test button (/gcbrun) for me please? :) |
/gcbrun |
matrix: | ||
k8s-version: | ||
- v1.22.x | ||
- v1.23.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to add newer kube versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL, oh dear :) Yes, done.
/gcbrun |
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
7ff1402
to
5177416
Compare
/gcbrun |
Signed-off-by: Ville Aikas vaikas@chainguard.dev
Fixes #2672
Here's the pieces for scaffolding and for createtree. I split them into two different directories because then applying just the scaffolding bits is easier and more well contained. Apply all the everythings :)
Createtree is a standalone job that makes creating trees in the k8s cluster easier and then because the entry is stuffed into configmap, it's easy to 'depend' on that for other jobs that may need it. For example, we have also a setup for spinning up a CTLog on top of this in sigstore, but wasn't sure if that might be of interest. I noticed that in README.md one of the steps after spinning up Trillian is to install CTLog on top of it.
https://github.com/sigstore/scaffolding/tree/main/config/ctlog
After creating the README-SCAFFOLDING.md in the examples dir, I noticed the PR template sez to add it to docs/ but I'm not sure if this belongs there since it's related to running tests, etc.
Lastly, the added kind test in github workflows only checks the configmap that the treeID was created, might be good to add some examples to test the grpc api as well, but figured that could be a followup if that makes sense.
Checklist