ContainerSource uses fixed names, rather than GenerateName#1628
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Harwayne The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
size/L
|
/test pull-knative-eventing-unit-tests |
|
Ping. |
| return ra, nil | ||
| } else if err != nil { | ||
| r.markDeployingAndRecordEvent(src, corev1.EventTypeWarning, "DeploymentGetFailed", "Error getting deployment: %v", err) | ||
|
|
| return nil, fmt.Errorf("getting deployment: %v", err) | ||
| } else if !metav1.IsControlledBy(ra, src) { | ||
| r.markDeployingAndRecordEvent(src, corev1.EventTypeWarning, "DeploymentNotOwned", "Deployment %q is not owned by this ContainerSource", ra.Name) | ||
| return nil, fmt.Errorf("deploymtn %q is not owned by ContainerSource %q", ra.Name, src.Name) |
| return ra, fmt.Errorf("updating deployment: %v", err) | ||
| } | ||
| return ra, nil | ||
| } else { |
There was a problem hiding this comment.
I think you can get rid of this last else statement and just log
There was a problem hiding this comment.
I don't think so. For example, if the receive adapter isn't found, then it is created. If we moved the log line outside the else, we would log that we are reusing an existing receive adapter.
| func (r *Reconciler) podSpecChanged(oldPodSpec corev1.PodSpec, newPodSpec corev1.PodSpec) bool { | ||
| // Since the Deployment spec has fields defaulted by the webhook, it won't | ||
| // be equal to expected. Use DeepDerivative to compare only the fields that | ||
| // are set in expected. |
There was a problem hiding this comment.
update comment, expected should be oldPodSpec?
There was a problem hiding this comment.
Done (switched to newPodSpec).
| func (r *Reconciler) createDeployment(ctx context.Context, source *v1alpha1.ContainerSource, args resources.ContainerArguments) (*appsv1.Deployment, error) { | ||
| deployment := resources.MakeDeployment(args) | ||
| return r.KubeClientSet.AppsV1().Deployments(source.Namespace).Create(deployment) | ||
| func (r *Reconciler) deleteOldReceiveAdapter(ctx context.Context, src *v1alpha1.ContainerSource, currentName string) error { |
There was a problem hiding this comment.
maybe add a TODO that this should be deleted after the 0.8 cut?
|
/lgtm /hold for cosmetic comments... |
|
The following is the coverage report on pkg/.
|
|
/hold cancel |
|
/lgtm |
|
/test pull-knative-eventing-unit-tests |
See openshift/release#26101 Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> Co-authored-by: Pierangelo Di Pilato <pierdipi@redhat.com>
5 participants
Helps with #1250
Proposed Changes
Release Note