propose proxy passdown for each language type #89
Conversation
enhancements/proxy-passdown.md
Outdated
| // Create []corev1.EnvVar from Operator environment | ||
| proxyVars := ReadProxyVarsFromEnv() | ||
| // Pass proxy vars to each container | ||
| for _, container := range dep.Spec.Template.Spec.Containers { |
There was a problem hiding this comment.
For the implementation, we should check dep for nil.
enhancements/proxy-passdown.md
Outdated
| include: | ||
| ```yaml | ||
| env: | ||
| HTTPS_PROXY: {{ lookup('env', 'HTTPS_PROXY') | default('', True) }}' |
There was a problem hiding this comment.
TODO: make it clear that this env is in the container spec of deployments/pods/etc and NOT a part of the Ansible env
There was a problem hiding this comment.
I don't think it will provide a good use experience of ansible/helm operator for customers. The author can only add one cluster proxy. And the bundle image can't be reused for other clusters. Why not use the way to add the cluster proxy of OLM. We shouldn't add the proxy config during the "build" operators. Should add the proxy config during the "run" operators.
There was a problem hiding this comment.
@emmajiafan, I've updated this example to show that the setting of the variables occurs during runtime (reconcile). If using OLM, the environment variables will be set on the operator deployment, which are passed down to the operands like in this example.
Since this takes place at runtime, I don't think we will have a problem reusing bundle images, though operator authors that want to enable multiple proxies would need to implement some additional code and not use the variables being passed to the operator by OLM via the operator deployment.
| including that functionality as a badge or part of the maturity | ||
| model on operatorhub, but that is out of scope for this proposal. | ||
| 2. Automatic scaffolding of proxy passdown cannot be done because the | ||
| logic must occur in operator-specific code. |
There was a problem hiding this comment.
Is it because the injected codes depend on the type of k8s resources being used by Operand instance? e.g. it could be a Deployment or a StatefulSet.
If that's the case, can we perhaps default scaffold out sample codes with Deployment and we provide other helpers for StatefulSet and document how to change to StatefulSet if Deployment is not what they want?
(I agree the scaffolding piece could come later or outside of this EP, just would like to know better if that's attainable later even if it's not for now.)
There was a problem hiding this comment.
Thats part of it. Here's what we currently scaffold for the go reconcile:
func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
_ = log.FromContext(ctx)
// your logic here
return ctrl.Result{}, nil
}AIUI our current strategy is to scaffold only the skeleton, and not to include example dummy code, so I think scaffolding a suggested best practice for one of many possible directions would be awkward, but I'm not opposed to it.
There was a problem hiding this comment.
What we can do instead is include this in the tutorial, which implements this function. https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/#reconcile-loop
operatorlib.AddProxyToDeployment(memcached) // pass along proxy env vars
There was a problem hiding this comment.
AIUI our current strategy is to scaffold only the skeleton, and not to include example dummy code, so I think scaffolding a suggested best practice for one of many possible directions would be awkward, but I'm not opposed to it.
Agree this is worthy of a broader scope discussion and not in the scope of this EP.
There was a problem hiding this comment.
What we can do instead is include this in the tutorial, which implements this function. https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/#reconcile-loop
operatorlib.AddProxyToDeployment(memcached) // pass along proxy env vars
Adding this to the tutorial to promote this sounds good to me, thanks!
| 2. operatorlib.AddProxyToDaemonSets(daem *appsv1.DaemonSet) | ||
| 3. operatorlib.AddProxyToReplicaSets(rep *appsv1.ReplicaSet) | ||
| 4. operatorlib.AddProxyToPods(pod *appsv1.Pod) | ||
|
|
There was a problem hiding this comment.
I've also seen StatefulSets are frequently used by the Operators for managing databases or stateful apps. Jobs and CronJobs could be good candidates.
enhancements/proxy-passdown.md
Outdated
| ## Risks | ||
|
|
||
| 1. We may not have captured all of the go utility methods, we may need to | ||
| add more helpers later. |
There was a problem hiding this comment.
Only see this after my previous comment. +1 on keep monitoring this and expanding the helpers.
enhancements/proxy-passdown.md
Outdated
| } | ||
| } | ||
|
|
||
| func AddProxyToDeployment(dep *appsv1.Deployment) { |
There was a problem hiding this comment.
There has been some discussion that perhaps adding proxy envs to appsv1-based objects is moving in the wrong direction, since this won't work with the future of Unstructured objects and server side apply. I'll talk about this with others and report back here. ReadProxyVarsFromEnv would still be useful.
There was a problem hiding this comment.
After discussing this, we have decided not to include the specific object helpers at this time. Instead, we will have docs and examples using the ReadProxyVarsFromEnv helper.
| ```go | ||
| // Read environment variables for "HTTP_PROXY, HTTPS_PROXY, and | ||
| // NO_PROXY" | ||
| func ReadProxyVarsFromEnv() ([]corev1.EnvVar) { |
There was a problem hiding this comment.
This function will need extensive godocs that include example usage.
There was a problem hiding this comment.
Link to this godoc from the FAQ with a new entry for proxy aware operators
Signed-off-by: austin <austin@redhat.com>
No description provided.