-
Notifications
You must be signed in to change notification settings - Fork 172
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
Define optimal multi-distro HA solution for the in-cluster registry #375
Comments
A. Require RWX StorageClass for HAPros:
Cons:
C. Use Statefulset combined with InitContainer for automatic image syncing in-clusterPros:
Cons:
Details:
Eliminated based on conversations with @runyontr / @YrrepNoj and comments below:
|
C seems to be the most elegant solution (even though custom stuff would need to be written). You could try C short term and then work with the Trow dev to improve that product. Relying on S3/minio for a core service is kinda a big con IMO. I have seen that go pretty poorly with minio in the past and a platform provided s3 is obviously not available on the other side of most air gaps. |
Thanks @Racer159! After chatting with some team members we are going to focus on options A - D for now. |
B. I think people will have the most familiarity operating in with this model. I also think IAM is not a con, it would merely need to be purposeful, which also alludes to multiple human people aware of the resource utilization. I also emphasize with the perspective that this inherently is a risk operationally if unable to modify account level IAM or being dependent on others. I go back to my initial takeaway that seat belts aren't always bad. In my view it is the most straight forward approach. Worth a test to gain insight on concerns of latency. I would also want to know how integral are the mechanics as compared to the functional requirement? Does the implementation greatly drive any subset of users one way or another? Just some thoughts... |
C or D would probably be the way I'd lean, with a preference toward C. I think the code that would be necessary to write would be simple enough and the benefit of the portability is great. Agree with @Racer159 on the S3/Minio comments in regards to B, and there could be edge cases with how to handle situations where perhaps the consumer may already have a HA storage solution setup in cluster and is now burdened with maintenance on another one. A could perhaps make sense with an optional deployable component that provides a minio deployment that satisfies the PVC requirement? Suppose that's somewhat of a hybrid of A+B |
Another piece of data--with zarf in a connected/semi-connected environment, HA is essential as not having HA would actually reduce fault-tolerance as it's very likely any other external registry would already be highly-available. |
Another idea to use IPFS instead of object storage has been mentioned and even some POCs might be worth considering, but I think those would have to be solutions down the road because they are still POC: https://github.com/ipdr/ipdr and a more direct POC https://github.com/joshrwolf/ripfs. |
As other's have stated C/D. The amount of overhead code that would be added seems well worth the portability you would gain. C is likely the more standard way to implement such a feature with D being its quicker/dirtier counterpart imo. @jeff-mccoy IPFS is a fantastic idea and is the HA of HAist things you could implement and you get all the great things that come with that tamper evidence, etc. As a nerd I'd prefer to see this as the end solution. Ultimately to solve the issues in the near term, C or D determined on the level of effort desired to complete the solution. Could always start with D and iterate to C. |
Not sure if this is the odd man out opinion, but I lean B or C. B makes sense to me because I'd like to think that MinIO would support the majority, if not all, of the deployment targets. I also like the idea of being able to leverage a mature helm chart for storage and their docs as well. C, if I'm understanding it correctly, sounds interesting. My concern with custom solutions like this becomes a question of "how difficult will it be to debug if it goes haywire?" An initContainer sounds more of a hassle to deal with than fixing values in a Helm chart that has documentation. |
@salt-mountain thanks for the feedback. Regarding C: I actually discussed this exact issue with another engineer last week. The leaning on initContainer for me made sense because I could use my normal K8s troubleshooting flow for debugging vs some custom orchestration behavior. I think it's fair to say we'd need to prove that out, but I would say either initContainer or a smart readinessProbe + waiting to start serving the registry until the registry is updated could work too. The biggest issue I see is not when we are actually updating images, but if a node fails, we will need that pod to spin up on another node and would not it to report ready until it was fully up-to-date. |
My .02 - although not adding any significant/new opinion. C sounds like a logical starting point for portability without added layers of complexity baked in. (IE MinIO/S3 or RWX compliant storage). That said I'd be curious about playing devils advocate for a solution that possibly enables a combination of A/C. I believe utilizing RWX storage if available would be optimal. Only sticking to C might be an unnecessary burden for those who have the infrastructure available. (Although if wouldn't be that big of a burden) I have seen orchestration that includes multiple configurations, which could be tailored to a case such as this - deployments w/ a RWX storage or statefulsets with a syncing mechanism. Lot's of details to stort there and I'm discluding the maintenance required to support multiple configs (and maybe a greater discussion about if that should be done at all). I thought it may be worth entertaining. I think we all know of many growing spaces that will require air-gap processes with cloud infrastructure available that would benefit from being able to use that infrastructure. |
Agree with C as best option. D would cause issues, as mentioned O(n). A,B require something to happen beyond zarf to meet the pre-reqs. For option A, what would Zarf do for the PVCs? |
Between B and C I think I'm more in favor of B, but C doesn't give me heartburn either. If we went with B we might be able to offer the ability to choose the in-cluster Minio or utilize external S3 if the user is capable of it. |
BLUF/summary of the wall of text:
Option B sounds terrible to me for the following reasons:
There's a lot to like about option A from a KISS perspective: (The big 3 CSPs now support RWX storage classes)
The one flaw of option A is bare metal:
When I first read option C & D, my gut told me those sound like clever hacky duct tape solutions, but there's some advantage to perusing them:
|
It's worth pointing out that you left out requirements in your original ask for input, being clear on the problem trying to be solved might help further narrow down choices. I'll give some example requirements gathering questions that might help make the point clearer:
An example of why these questions matter:
|
Thanks @neoakris. This registry is the docker registry that zarf installs on a
For single node deployments (we call them appliance mode), I agree you certainly could just keep all your zarf packages and destroy/recreate and that might be sufficient. So that should be a consideration, especially since by default we use k3s and the local-path-provisioner, which is really just a folder on disk, so if the nodes dies we probably have bigger problems. The registry will need to be long-lived (as long as the cluster lives) and the default deployment does not expose the registry for consumer use, though could be exposed with an ingress--we just don't do that in the standard configuration. The registry is meant to serve the cluster it is deployed to. Kube-fledged is an interesting solution, but solves a slightly different problem. Much like Kraken you still need some form of reliable registry as a source-of-truth. It is also a good bit more complex, increases storage needs by caching images on every node and only has one main developer based out of India, which may be a problem for some of our US Dept of Defense users unfortunately. |
Regarding your final point on multiple non-HA registries, that's another option too I believe (and basically what option D is trying to do), but my concern remains around what happens when a node dies and the pod spins back up on a new node with no existing registry data. |
Between C and D, D actually feels more duck-tape like to me. C would leverage the OCI distribution spec to sync registries that def. battle-tested and use everywhere in prod. The initContainer component could be changed to some smarter init steps on the main container too, I just like the idea of using the initContainer instead because it makes it very easy to see where things are in the rollover process and where an issue exists with normal K8s tooling. |
Moved option D to eliminated based on discussions above. Going to eliminate option B as well: B concerns me for the external dependencies that are (ironically) more complex for S3 vs EFS and not universal across the cloud providers. Minio also concerns me because of some of the reasons stated above: complexity of the deployment/upgrade, concern with conflicting other deployments of Minio, overall size, troubleshooting issues. Looking at option A I do think it is nice for the fact that we can basically push the problem to someone else, but I also think we lose a lot by no longer just requiring a KUBECONTEXT. Quick googlefoo of the 3 major cloud providers indicate additional IaC or manual steps required on the cloud-provider side before those options will work. Note AKS does have an HA NFS v4.1 option that doesn't seem to require additional configuration. https://cloud.google.com/kubernetes-engine/docs/how-to/persistent-volumes/filestore-csi-driver Since we have the ability to override storageclass now during init, I think A could be a great override behavior. Someone on LInkedIn mentioned NFSv4 as a good candidate as it's already supported by K8s, but HA NFS still looks pretty complicated. That leaves us with a combo of C with an override for A. We could also explore IPFS for this, but I think that's another layer of complexity right now with a lot of unknowns around Day 2 operations. |
sounds like good analysis; however, before you completely throw out option D. (multiple non-HA registries being load balanced.) multiple non-HA registries + LB = dead simple KISS, reliable, and maintainable.
This may be worth considering:
Otherwise you may have narrowed down to option C as a universal solution. |
Some of this has already been mentioned, but I wanted to highlight that: HA is more than just object storage, and please consider the risk and complexity of an in-cluster implementation.
I currently use ECR and mirror images for a Big Bang installation to it. It has generally been smooth. I've implemented an out-of-cluster registry once for an on-premise proof of concept before. One note: when doing Cluster API, you'll have a management cluster in addition to the main cluster that also needs a registry. It's simpler if these can share a registry (but not required). My use case was proof of concept and did not use a production quality registry. |
I was doing some reading on the replication process implemented by openebs.
It may not be applicable - but the replication process might provide insight into a future implementation? or it might not and this can be disregarded 😄 . |
Looking through this I'll vote along side @Racer159 C seems elegant, simple and the only con is we do work. I'm ok with us doing work. I also feel like going the A direction is going to put a lot of work on the user, and will likely lead to more debugging down the line of the type of "My does funny things with Zarf, HELP!" |
…754) ## Description This PR introduces the ability to connect to an already existing (and reachable) Container Registry and/or Git Repository during the `zarf init` command. Closes #570 (Support using an external git server) Closes #560 (Support using an external registry) This implementation will serve as a good midway point on having a fully HA in-cluster registry #375. ## PR Feature List - Added several flags to the `init` command to support using an external git repository - Added several flags to the `init` command to support using an external container registry - Update `zarf connect registry` to direct to `{HOST}/v2/_catalog` (this was confusing some other people since it would originally seem like the registry was returning an empty page) - Add utility function to create a tunnel to a service URL - Created slightly better regexp for replacing the host from a `containerImage` url - semi-refactored the `zarf package deploy` logic ## Breaking Changes List - We are changing the structure of the names of repos & containers we are pushing (we are simplifying the name and adding a sha1 hash of the original name to the end of the name) Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com> Co-authored-by: Megamind <882485+jeff-mccoy@users.noreply.github.com>
…754) ## Description This PR introduces the ability to connect to an already existing (and reachable) Container Registry and/or Git Repository during the `zarf init` command. Closes #570 (Support using an external git server) Closes #560 (Support using an external registry) This implementation will serve as a good midway point on having a fully HA in-cluster registry #375. ## PR Feature List - Added several flags to the `init` command to support using an external git repository - Added several flags to the `init` command to support using an external container registry - Update `zarf connect registry` to direct to `{HOST}/v2/_catalog` (this was confusing some other people since it would originally seem like the registry was returning an empty page) - Add utility function to create a tunnel to a service URL - Created slightly better regexp for replacing the host from a `containerImage` url - semi-refactored the `zarf package deploy` logic ## Breaking Changes List - We are changing the structure of the names of repos & containers we are pushing (we are simplifying the name and adding a sha1 hash of the original name to the end of the name) Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com> Co-authored-by: Megamind <882485+jeff-mccoy@users.noreply.github.com>
Inline with @RothAndrew's comment:
I've just submitted PR #1664 which does exactly this; disabled HPA by default, and when it is enabled, requires a RWX-compatible StorageClass for the PVC (we just create our own and pass the name to Zarf Init). Our team identified an issue where the Zarf Registry doesn't scale when doing Node AMI upgrades, which led to the contribution towards this effort. |
HPA does not require RWX for node-attached storage. What storage provider was requiring RWX? |
The PDB for the Zarf Registry requires a pod available at all times, but when a rolling node update takes place (for example upgrading EKS node AMIs), an additional registry pod cannot be created on a different node because RWO only allows same-node access. We’re using EFS to solve this problem by allowing multiple Registry pods to access the same volume - which required RWX for the PVC |
## Description When attempting to upgrade our EKS Node AMIs in AWS, we noticed that the Zarf Registry deployment was unable to horizontally scale across nodes which needed to restart. We believe the culprit is the `accessMode` specification for the PersistentVolumeController. In order for multiple pods to have access to the same PersistentVolume, the `accessMode` must be set to "ReadWriteMany". ~~This PR proposes that when autoscaling is enabled for the Zarf Registry, the `accessMode` is set to "ReadWriteMany" by default; when autoscaling is disabled, it is set to "ReadWriteOnce". Due to the additional work required (i.e. using an existing PersistencVolumeController with a storage class compatible with RWX), we also propose that `autoscaling` be disabled by default.~~ This PR exposes the `REGISTRY_PVC_ACCESS_MODE` variable for the `zarf-registry` portion of the init package. ## Related Issue - Relates to #375 ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed
## Description - Provided fields for affinity and toleration of the registry pod - Added s3 irsa configurability for the docker registry helm chart if using a compatible image ## Related Issue Relates to #375 ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/.github/CONTRIBUTING.md#developer-workflow) followed --------- Co-authored-by: Zack A <zack-is-cool@users.noreply.github.com> Co-authored-by: corang <jordan@defenseunicorns.com> Co-authored-by: Zack Annexstein <zannexstein@gmail.com> Co-authored-by: Lucas Rodriguez <lucas.rodriguez@defenseunicorns.com> Co-authored-by: Lucas Rodriguez <lucas.rodriguez9616@gmail.com> Co-authored-by: razzle <razzle@defenseunicorns.com> Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com>
Currently Zarf uses a helm chart to deploy a docker registry. This chart utilizes deployments with an optional PVC for persistence. We need to define a better solution for making the registry highly-available and fault-tolerant. This issue exists to facilitate that discussion.
Concerns:
Considerations:
The text was updated successfully, but these errors were encountered: