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

Update k8s libs #56

Closed
wants to merge 766 commits into from
Closed

Conversation

george-angel
Copy link

We cannot update all, because of the gnostic bug:
google/gnostic#262, but we can update the k8s
deps at least

alkar and others added 30 commits January 7, 2021 12:34
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
This is the proper name of the kubernetes resource.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
- We only need the token. We are always operating inside the same
  cluster so the CA certificate and server address do not change.
- Uses existing plumbing with ApplyFlags.
- Leads to reduced code complexity

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Add delegateServiceAccountSecretRef field to the CRD
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Co-authored-by: Rob Best <robertbest89@gmail.com>
This works well with the client namespace base provided.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Use a default for delegateServiceAccountSecretRef
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Since we're providing a default value, this should not longer be
required. Plus, a required field with a default value doesn't quite
work.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Although an empty value will simply produce an error because you can't
have a resource named "", it is likely more user-friendly to surface the
issue immediately and it is more efficient since kube-applier will not
have to operate on this Waybill only for it to eventually fail.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
It no longer needs to be a *string

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
It was erroneously changed to a *string but it doesn't make sense.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Make delegateServiceAccountSecretRef optional
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
The duration is in seconds and the format %.3f does not make sense.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Change duration format in status page
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Errors during the setup steps of a run request are currently logged by
kube-applier but not presented to the user. The implementation of
setRequestFailure() is incomplete and so they are never presented to the
user.

These errors that do not originate from kubectl better fit in the
concept of a kubernetes event. Instead of updating the status
subresource of the Waybill, we can instead emit events. This will
maintain the information of the actual last run in the status of the
Waybill but still surface the issue to the user.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
Capture run setup errors in kubernetes events
If left empty, it will default to the name of the namespace where the
Waybill resides.

Signed-off-by: Dimitrios Karagiannis <dhkarag@gmail.com>
ribbybibby and others added 25 commits August 20, 2021 10:05
0.2.1 removes support for v2 headers, which is a breaking change if you
are using secrets encrypted with the short-lived v2 header format.

Reverting for now so I can push other changes from 3.3.5 out in a 3.3.6
release.
- Build on push to any branch
- Move tests back into the docker build. We need the strongbox binary to
  perform the tests and by doing it in the docker build we're assured
  we're using the same version without having to maintain two separate
  version strings in the Dockerfile and the GH actions config.
- Checkout with depth 0. The tests rely on having history to perform a diff on.
The minute-long delay on the status being updated can be quite
frustrating when you're force-running things and waiting for results to
appear.

I think it's better to return the status of the waybills to the user
from Kube when they request them.

The load on the apiserver should be mitigated by the use of a caching Kube
client, which controller-runtime provides and I will implement in a
separate commit.
This client will read from a cache and only write directly to the API
server. The cache is populated by a watch.

As part of this, I've also tweaked our client to use the
EventBroadcaster provided by controller-runtime too.
webserver: retrieve Waybills on request
A new Waybill won't appear on the UI if there's some kind of setup error (i.e a
problem fetching the delegate secret) or if it's created with
`autoApply: false` because there's no 'LastRun' status field.

I've had a few reports from users where this has created confusion, so
I've added a panel that shows 'Pending' Waybills that are yet to have
their first run.

In addition to this, I've also added events to the UI, which should give
users a decent amount of context to help them understand what's going wrong and
maybe fix it themselves without having to contact us.
webserver: add panel for Waybills without a last run
Required to list and cache Waybill events.
If you're applying a Waybill with itself (which is what we do) then you
can run into a situation where:
1. KA retrieves the Waybill and starts a run
1. During the run, `kubectl apply` updates the Waybill
2. An error is produced when KA attempts to update the Waybill's status
   at the end of the run because 'the object has been modified' (because
   `kubectl` has changed it)

To mitigate this, get the latest version of the Waybill before updating
the status.

Also add an event when status updates fail (for other reasons) so there's
some information on the UI/in kube for the user as to why their manifests
seem to have been applied but the status hasn't been updated.
runner: get latest version of Waybill before status update
YAML anchors are once again supported, and enabled by default
We cannot update all, because of the gnostic bug:
google/gnostic#262, but we can update the k8s
deps at least
@CLAassistant
Copy link

CLAassistant commented Oct 29, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ ribbybibby
❌ george-angel
You have signed the CLA already but the status is still pending? Let us recheck it.

@george-angel george-angel deleted the update-k8s-lib branch October 29, 2021 08:34
@george-angel george-angel restored the update-k8s-lib branch October 29, 2021 08:35
@george-angel george-angel deleted the update-k8s-lib branch October 29, 2021 08:43
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.

None yet

5 participants