knative / operator

Combined operator for Knative.
Apache License 2.0
179 stars 98 forks source link

Update `net-kourier-controller` container name from `controller` to `kourier-controller` #1667

Open lou-lan opened 5 months ago

lou-lan commented 5 months ago

Problem

We want use spec.registry.override to replace image.

The operator uses the container name as the logic for replacement, with both the net-kourier-controller and the knative-releases/knative.dev/serving/cmd/controller container names being controller. This will cause two different applications to be replaced with the same image.

apiVersion: operator.knative.dev/v1beta1
kind: KnativeServing
metadata:
  name: knative-serving
  namespace: knative-serving
spec:
  registry:
    override:
      "kourier-gateway": "docker.reg-name.com/envoyproxy/envoy:v1.25-latest"
      "activator": "reg-name.com/gcr.io/knative-releases/knative.dev/serving/cmd/activator:v1.12.2"
      "autoscaler": "reg-name.com/gcr.io/knative-releases/knative.dev/serving/cmd/autoscaler:v1.12.2"
      "autoscaler-hpa": "reg-name.com/gcr.io/knative-releases/knative.dev/serving/cmd/autoscaler-hpa:v1.12.2"
      "controller": "reg-name.com/gcr.io/knative-releases/knative.dev/serving/cmd/controller:v1.12.2"
      "webhook": "reg-name.com/gcr.io/knative-releases/knative.dev/serving/cmd/webhook:v1.12.2"
  ingress:
    kourier:
      enabled: true
  config:
    network:
      ingress-class: "kourier.ingress.networking.knative.dev"
kubectl get pods  -n knative-serving  net-kourier-controller-655795785b-lvwxh -o yaml | grep 'name: controller'
    name: controller
    name: controller

Persona: Which persona is this feature for?

Exit Criteria A measurable (binary) test that would indicate that the problem has been resolved.

Time Estimate (optional): 0.1 day, I can perform the fix, but I need guidance on the location of the changes.

Additional context (optional) Add any other context about the feature request here.

kumarankit999 commented 5 months ago

/assign

kumarankit999 commented 5 months ago

Hey @lou-lan, thanks for raising the issue. Can you refer to the link of the page from where the issue is relevant

lou-lan commented 5 months ago

Hey @lou-lan, thanks for raising the issue. Can you refer to the link of the page from where the issue is relevant

Install by using the Knative Operator - Knative

bmendoza820 commented 3 months ago

@kumarankit999 My company is evaluating knative-operator and we are running into this same issue. We currently deploy knative-serving with kourier from a private container registry where images are scanned. Currently the operator we cannot manually override the net-kourier-controller image since there is no way to target the container.

Looking at https://github.com/knative/operator/blob/e35a88d01b9776c5d45aece5aabc1766c18cb14b/pkg/reconciler/common/images.go#L107, it appears the object name must be targeted, but this wouldn't work for pods generated from deployments or jobs since names are generated.

I'd be happy to discuss options and make a PR with the fix.

bmendoza820 commented 3 months ago

Actually, this comment in a related issue https://github.com/knative/operator/issues/736#issuecomment-907767938 points out that the deployment name should be used. I think this should be documented more explicitly. @lou-lan you can use net-kourier-controller/controller to target the container.

github-actions[bot] commented 1 week ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.