cloudnativesdwan / cnwan-operator Goto Github PK
View Code? Open in Web Editor NEWRegister your Kubernetes Services to a service registry of your choice and manage them automatically.
License: Apache License 2.0
Register your Kubernetes Services to a service registry of your choice and manage them automatically.
License: Apache License 2.0
Right now we are create a Kubernetes Secret
with Google's service account as its content and we mount it in the operator's pod's container.
This was fine until now, but with the support of etcd this is not a scalable solution anymore:
Mounting the settings file is fine because that never changes and we always expect it and we expect it to be the same way.
But this is no longer a good solution for credentials.
With etcd we are using a different approach: according to the authentication method, a Secret
must still be created of course, but now it's not mounted anymore, as the operator will pull the Secret
from Kubernetes on its own.
Service directory will experience the same thing: user creates the Secret
with their service account and it won't be mounted, but rather it will be pulled by the operator automatically.
Maybe we should add these steps as a script to scripts/etcd_init.sh
or similar...? Anything that decreases friction and gets the user up and running quickly is great! :D
Originally posted by @ljakab in #25 (comment)
Improve the way to make the operator monitor namespaces with an easier, more "semantic" and less error-prone way.
Note: this is not an urgent issue, but can improve usability of the operator. Code-wise it won't be hard to change, but depending on how users view the current system (easy or confusing) this maybe long or short term.
An administrator/user can define which namespaces the operator can monitor by putting namespaces inside an allowlist or blocklist.
If settings are set as listPolicy: allowlist
then the operator will only monitor namespaces that have label operator.cnwan.io/allowed: yes
(but really the value can be anything, more on this later).
On the contrary, listPolicy: blocklist
will make the operator monitor all namespaces and avoid those that have label operator.cnwan.io/blocked: yes
(again, the value can really be anything here).
This system is very convenient but can certainly be improved, as it has some shady parts that can be considered bad practice or misleading.
We have two labels to filter a namespace (maybe filter is not the right word here, but it explains weel), instead of one. What happens when a user - by mistake - puts both labels operator.cnwan.io/allowed
and operator.cnwan.io/blocked
in the same namespace? The operator will know which one is the one that it has two check. But nonetheless this can confuse users, who can potentially mis-configure the operator, which in turn can have unwanted/unexpected behaviors.
operator.cnwan.io/allowed: no
. But this will have no effect as the key is actually the important part (read next point) and will bring even more confusion.operator.cnwan.io/allowed: yes
and operator.cnwan.io/blocked: yes
are a bad use of labels and a bad practice as a consequence: in labels and metadata what is actually meaningful is the value, not the key. They key is supposed to be the differentiator and holder (after all, you can think of it as a YAML key), while the value is supposed be the important thing because it tells how key must be configured. Instead, we are actually considering the key here (../allowed or ../blocked) as the actual value, which is a bad practice because it is not what its job is.
yes
or true
is useless, because in that case it might just as well not be there entirely. Additionally, I think it would be pointless to have a key that can have only two values (synonyms) and the others are "invalid" (for lack of other word) in light of the previous sentence.Minor thing as it is just a language issue: but the operator is not really allowed or blocked to do anything here so it is just a minor confusion point.
The current way to set the default list as listPolicy: allowlist/blocklist
is good and can even stay like this, but I think that it can be improved and made easier to understand.
listPolicy: allowlist
may be wrongly interpreted as "namespaces are in the allowlist by default" if one does not read the documentation.I intend/propose to change the operator to make it better and more understandable with following ways.
The listPolicy
setting can be moved to a better description.
For example: listPolicy
can be changed to defaultBehavior
or defaultAction
to make it clear that it modifies the default way the operator works.
listPolicy: allowlist
can be changed to defaultBehavior: ignore
which makes the operator ignore (I think this is a better fitting word) a namespace by default.listPolicy: blocklist
can be changed to defaultBehavior: watch
which makes the operator watch (again, better fitting word) a namespace by default.This is more pressing issue.
Instead of having two different keys, have only one that is more good practice: operator.cnwan.io/action
. The prefix operator.cnwan.io
again proves that what comes next (action
) only makes sense for the cnwan operator (just as k8s.io/something
makes sense for kubernetes internal purposes).
To watch a namespace use values watch
, include
, enable
and maybe something else. I think that operator.cnwan.io/action: watch
is more fitting and better understandable. watch
is of course enough, but multiple synonyms allow a user not to have to look at the documentation if they don't remember the valid word.
On the contrary, ignore
, disable
, exclude
, etc. will replace operator.cnwan.io/blocked
.
For people that used versions where the old names were used, an upgrade.sh
shell be provided that will automatically write the correct values and change the labels accordingly.
A DEPRECATION NOTICE will be included in the documentation.
The old mechanics will be supported for x (like, 2?) major version cycles with the DEPRECATED
notice.
Recently, the golang package for service directory has been updated to v1
from v1beta1
(what we are using now).
The operator will be updated to include this change. Since this is not critical, it will probably come within other PRs.
At the beginning of the Operator project I didn't really find a reason to create probes for the operator -- e.g. readiness probes and/or liveness probes. But now, since much effort is going to be done to improve the operator and move away from some component of kube-builder and operators in general, I am thinking of creating probes to the operator.
For example, since the operator will not have dependencies on mounted files, the operator will be able to to start even without any settings or secrets deployed to kubernetes and it will not crash (what it does now), but will merely wait for it to happen, sending not ready values to the probes. This will make the operator more flexible as it will wait for users to deploy the needed file and this does happen the probe will receive a ready state. The normal behavior will occur, which is that after x consecutive not ready values, the operator will be restarted, which currently happens immediately right now.
There are many other use cases, which I will show when this is implemented.
The Operator requires a particular version of K8s that should be documented. It also requires as of now access to GCP public APIs, which needs to be taken into account specially in private deployments behind a proxy. These should be documented.
Installing the operator ideally should be as simple as helm install cnwan-operator
. We should create a Helm chart that:
etcd
using the bitnami/etcd
chart, with sane defaults, but allowing customization of the most critical valuesI will start experimenting with Ingress resources, so that we could start supporting it, since it makes sense for many users to use an ingress to expose their services inside the cluster. And because it's about time :)
The workflow from https://github.com/CloudNativeSDWAN/cnwan-operator/blob/master/.github/workflows/publish-github-package.yml pushes to GitHub Packages with the tag created from the version number. In addition to that it should also tag the same image as latest
.
Instead of providing secrets with tokens, the operator could use AWS's v2 SDK (which is already using) for logging in, in case it is running on EKS.
For reference: https://blog.jimmyray.io/kubernetes-workload-identity-with-aws-sdk-for-go-v2-927d2f258057
Right now, we use go version 1.13
.
A new update will make the operator use 1.15
and will re-run init to clean a lot dependencies.
On next versions, dependency from Viper will be completely removed in favor of local settings passed by value and only the ones that are actually needed by the package/struct.
Currently the project uses version v0.6.3
of controller runtime, which is 1.5 years old.
Updating controller runtime to a more recent version requires many changes to our controllers as versions after v0.6.3
introduce breaking changes.
As such, the controllers will be revised and updated in order to utilize a recent version of the controller runtime (ideally always the latest of course).
Github has a feature to do Auto-release notes.
Need to look into this but I think that it means that a PR can be opened and when tagging you can link that PR to automatically get release notes.
To better reflect its status as beta
, versioning will be changed from current 2.0.0
to 0.<something>.0
.
While making tests today I realized that Cloud Map treats tags as incremental, so if an object has tags a=1
and you want to update them as a=1;b=2
it will work fine. But if you want to update them as b=3;
then you will first have to explicitly call a delete operation for a=1
, as that will not be removed with an update only.
This is not really urgent and is being fixed in the new API but will be fixed in the operator as well if request is there as this will inevitably mean implementing a diff algorithm, which -- as of current state of code -- could mean having to remove everything and re-adding everything again.
When etcd package will be merged, we will have public packages that will be used by other repositories in a convenient way (first example: the cnwan reader).
So, public packages will undergo some modifications (in some cases very lightly) to make them more usable and organized and reach some kind of a v1
for them. These changes are heavily inspired by other stable packages.
This is just a draft of how they will be
.
└── pkg
├── serviceregistry
│ ├── core
│ ├── broker
│ ├── etcd
│ ├── gcp
│ │ └── servicedirectory
│ ├── aws
│ │ └── cloudmap
│ └── coredns
├── controllers
├── internal
└── settings
Package path | Description |
---|---|
pkg |
This is where all packages will live. Having them under pkg is a best practice in golang. NOTE: I will move controllers here as well. |
pkg/serviceregistry |
Main package that contains all the others. Contains no code and only has a doc.go that documents the package as a whole for pkg.go.dev . Right now it is called servregistry but will be called serviceregistry with this change. |
pkg/serviceregistry/core |
Contains just the definitions: the ServiceRegistry interface, the KeyBuilder interface, and all the objects (Namespace, Service and Endpoint), common errors and a new error type which will be used by all other subpackages here. No implementation. All functions will have full names, i.e. CreateEndpoint instead of CreateEndp , and will use short names as golang suggests only for internal and not exposed stuff. |
pkg/serviceregistry/broker |
Broker will be promoted to its own package |
pkg/serviceregistry/etcd |
Implementation of ServiceRegistry with etcd |
pkg/serviceregistry/gcp |
no code package, just a container for sub packages. i.e Service Directory and maybe dns with service directory or something |
pkg/serviceregistry/gcp/servicedirectory |
implementation of ServiceRegistry with service directory |
etc.... |
On next versions, more descriptive and idiomatic errors will be implemented for all packages, probably with custom structs that inherit from errors
. This will make it much more easier to understand why a feature didn't work and what object experienced the error.
Right now, there is an issue that doesn't allow the operator to understand when an error is due to time out expiration. The operator can still terminate a call due to time out, but when it checks the error to see if the timeout expired, that check never happens because of mis-matched types. In fact this mostly happen when grpc, because it encapsulates the error and thus is not of type context deadline exceeded
anymore.
This is not a critical bug but will be fixed on next versions anyway.
as per title (this is just a reminder for next commits)
Currently, settings for the operator have cnwan.io/profile
as default allowed annotations.
A new update will remove this, leaving the user complete choice during deployment phase.
The workflow from https://github.com/CloudNativeSDWAN/cnwan-operator/blob/master/.github/workflows/push_image.yml pushes to Docker Hub with the tag created from the version number. In addition to that it should also tag the same image as latest
.
Currently, the Operator takes the name of the namespace in k8s and uses it as the namespace for the service registry. It might be interesting to have a configuration option that would allow to specify which k8s namespace maps to which service registry namespace.
govulncheck
is a new tool released by the go team that analyses the code for known vulnerabilities. I will check this and see if we can integrate it on our CI.
For reference: https://go.dev/blog/vuln
Images cannot be built & published correctly on DockerHub because the repository name and namespace is pulled from a secret, and it is converted as ***/cnwan-operator:something
.
I'm going to fix it by turning it into an environment and use secrets just for authentication.
Please don't create any releases/tags while this is broken.
Right now, the operator has basically all permission for the resources it needs.
This is not accurate as it never actually performs some actions on Kubernetes.
For example: the operator has permissions to create or delete namespaces, although this is something that it never does.
The operator will be updated with only the correct permissions.
After that, permissions will be added as features are introduced, not upfront.
Note that this doesn't change anything in the way that the operator works, so its semantic version will not be updated.
Go here: https://github.com/CloudNativeSDWAN/cnwan-operator/blob/master/docs/etcd/quickstart.md#2---deploy-a-test-service and search for this line
Please notice that the namespace has this label:
operator.cnwan.io/watch: enabled
which inserts the namespace in the opeartor's allowlist. Also notice that the service has annotations that will be registered as metadata:
This link: ./concepts.md#namespace-lists does not really exist but the link check workflow is not catching this, probably because it doesn't work well with anchors :(
While testing on a GKE cluster the auto network detection -- for the Cloud Metadata feature -- I realized that the current way to get them is deprecated, as you can see here https://pkg.go.dev/google.golang.org/[email protected]/container/v1#ProjectsZonesClustersService.Get
This will be fixed, maybe in #98.
The code of the operator will be completely rethought and reorganized.
The update can be thought of as a new major version that will bring enhancements, fixes and stronger testing, among other fixes.
A full list will be included in the pull request as well.
Since new users testing the operator already have a Kubernetes cluster, it's easier if etc is installed into the same cluster rather than a separate machine. Add a quickstart style documentation for doing just that, with the caveat that it won't be a secure by default installation.
A new exiting mechanism will be implemented.
Currently we inherit os.Exit(n)
from Kube-builder which is fine, but it does not honor defers
and is too annoying to provide exit codes.
Coming on next version but not critical.
Update: this will probably use custom errors since it basically asks for an implementation like that, so it may be done with #28
As per title.
The latest versions of Kubernetes graduated to GA the new IPv6 and dual-stack services and the operator will need to support this change.
Though not urgent, this feature will come on a future version.
As per title, the operator's container image can/should be signed: https://docs.docker.com/engine/security/trust/
It would be interesting to have examples in the documentation that tie with the rest of the CNWAN project. In particular I think we should have some examples that use "cnwan.io/traffic-profile" through the documentation. That would be specially useful for those that come to the project after watching some CNWAN demo.
README
will be updated with a list of components that must be installed prior to deploying the operator.
Additionally, some sections will be improved, i.e. by changing the title to a more descriptive one and moving them to follow a more natural installation/deploying flow.
Pull Request #3 brings the aforementioned changes to the project.
As part of the CN-WAN presentation in KubeCon NA, we raised the question: "Should Kubernetes have semantics in general for traffic profiles?".
Our motivation for asking this question comes from our experience with CN-WAN. We have found really helpful to have annotations that specify the traffic profile for an application (i.e. the type of traffic that the application generates or consumes). In the CN-WAN case, those traffic profiles help the CN-WAN components to optimize the application traffic over the WAN connection. We wonder if the model could be generalized and apply to Kubernetes applications in general, so that application traffic can be optimized not only on the WAN but in any network (e.g. cloud, DC, campus, etc) based on the traffic profile.
We would love to hear opinions on this idea, so we are opening this GitHub issue to collect the thoughts from the community.
Once PR #43 is merged, adopt the new code to get GCP service account from the Kubernetes secret, and avoid mounting it into the pod.
Explore publish the CNWAN Operator in OperatorHub and update the repo as needed. Update documentation to describe the process of installing the CNWAN Operator through OperatorHub.
Current Operator logs the removal of services from the Service Registry as follows:
2021-02-11T18:38:59.492Z DEBUG ServiceRegistryBroker.RemoveServ going to remove service from service registry {"serv-name": "web-training"}
However, it would be very interesting to get some extra context on why the Operator is removing the service.
Just as it currently happens with the Cloud Metadata feature, where the operator automatically detects the platform, network and sub-network where it is running -- e.g. GKE, a new feature will be introduced that automatically infers the Region and Project ID in case the cluster is GKE.
To do this write auto
as a value, e.g.:
projectID: auto
defaultRegion: auto
I'd prefer to do it with an explicit auto
rather than keeping them empty and just try it anyways, but I will welcome different opinions and feedback! :)
When creating a service of type LoadBalancer
in EKS, this service is assigned a hostname. See example:
[truncated for brevity...]
status:
loadBalancer:
ingress:
- hostname: abc123674efg-648438329.us-west-1.elb.amazonaws.com
Obviously this won't be accepted by either Cloud Map or Service Directory, as they will only accept IP addresses, whether v4 or v6. So, in conclusion, whenever we see hostnames instead of IPs we need to resolve them in some way.
Specifically, for this scenario (EKS) I am thinking of these two ways of working:
Please provide ideas/comments/feedback if you think/know of an alternative way, as they are more than welcome!
Will keep posted.
Different service registries have different method to store an object and index it.
For example:
locations/<location>/projects/<project-id>/namespace/<namespace/etc/<etc.>
/prefix/namespaces/<namespace>/etc/<etc.>
my-endpoint.my-service.my-namespace.cnwan.local
aws:doeq9431ns
if I recall correctly.While knowing the ID
of the resource may not be useful for end users - i.e. reader or adaptor, they are necessary for other entities like the operator's broker because it needs to find them.
An update to the operator's service registry package will be brought which also includes an ID string
field, which differs from the already existing Name string
as the former will make sense for the specific service registry, while the latter for the user.
From the update on, Name
will only contain the user-friendly name of the resource and what we are currently referring to as Name
will be moved to ID
.
Other names for ID
:
ResourceName
ETCD is a distributed key-value store and having moved from incubation to graduation for CNCF it makes sense to leverage on it to create a sort of reliable "in-house" service registry.
A PR will come that will enable the Operator to use etcd as a service registry.
Advanced installation is mostly useless: most people that want to contribute already know how to do that as they probably know Kubernetes and that the simple installation method is actually way more than enough even for them.
Its existence is basically only a burden, as it is just one more thing to support and update while being rarely used. This last point is also more valid since we're about to have helm installation.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.