Skip to content
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

WIP: Add DesiredStateChanged event filter #110

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bobh66
Copy link
Contributor

@bobh66 bobh66 commented Mar 31, 2023

Description of your changes

Added the DesiredStateChanged EventFilter to the controller to avoid unnecessary reconciliations.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Executed this code in a development cluster doing multiple deployments.

@bobh66 bobh66 changed the title Add DesiredStateChanged event filter WIP: Add DesiredStateChanged event filter Apr 4, 2023
@bobh66
Copy link
Contributor Author

bobh66 commented Apr 4, 2023

Further testing has revealed a possible issue with Objects not getting updated properly. I'm investigating.

@chlunde
Copy link
Contributor

chlunde commented Jun 27, 2023

@bobh66 did you find anything here? for example, if an immutable object is attempted changed, provider-kubernetes "spins" very actively trying to reconcile it :( I think this would fix that

  Type     Reason                        Age                      From                                     Message
  ----     ------                        ----                     ----                                     -------
  Normal   CreatedExternalResource       20m                      managed/object.kubernetes.crossplane.io  Successfully requested creation 
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  8m39s                    managed/object.kubernetes.crossplane.io  cannot apply object: cannot patc
  Warning  CannotUpdateExternalResource  3m38s (x849 over 8m39s)  managed/object.kubernetes.crossplane.io  (combined from similar events): 

full event (example):

  Warning  CannotUpdateExternalResource  4m38s (x849 over 9m39s)  managed/object.kubernetes.crossplane.io  (combined from similar events): cannot apply object: cannot patch object: Job.batch "pi-1" is invalid: spec.template: Invalid value: core.PodTemplateSpec{ObjectMeta:v1.ObjectMeta{Name:"", GenerateName:"", Namespace:"", SelfLink:"", UID:"", ResourceVersion:"", Generation:0, CreationTimestamp:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), DeletionTimestamp:<nil>, DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string{"controller-uid":"030305ca-7ab7-4d05-8c7c-e3cb92e7588f", "job-name":"pi-1"}, Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ManagedFields:[]v1.ManagedFieldsEntry(nil)}, Spec:core.PodSpec{Volumes:[]core.Volume(nil), InitContainers:[]core.Container(nil), Containers:[]core.Container{core.Container{Name:"pi", Image:"perl:5.34.0", Command:[]string{"perl", "-Mbignum=bpi", "-wle", "print bpi(1) second2"}, Args:[]string(nil), WorkingDir:"", Ports:[]core.ContainerPort(nil), EnvFrom:[]core.EnvFromSource(nil), Env:[]core.EnvVar(nil), Resources:core.ResourceRequirements{Limits:core.ResourceList(nil), Requests:core.ResourceList(nil), Claims:[]core.ResourceClaim(nil)}, VolumeMounts:[]core.VolumeMount(nil), VolumeDevices:[]core.VolumeDevice(nil), LivenessProbe:(*core.Probe)(nil), ReadinessProbe:(*core.Probe)(nil), StartupProbe:(*core.Probe)(nil), Lifecycle:(*core.Lifecycle)(nil), TerminationMessagePath:"/dev/termination-log", TerminationMessagePolicy:"File", ImagePullPolicy:"IfNotPresent", SecurityContext:(*core.SecurityContext)(nil), Stdin:false, StdinOnce:false, TTY:false}}, EphemeralContainers:[]core.EphemeralContainer(nil), RestartPolicy:"Never", TerminationGracePeriodSeconds:(*int64)(0x4012805d98), ActiveDeadlineSeconds:(*int64)(nil), DNSPolicy:"ClusterFirst", NodeSelector:map[string]string(nil), ServiceAccountName:"", AutomountServiceAccountToken:(*bool)(nil), NodeName:"", SecurityContext:(*core.PodSecurityContext)(0x4017a80750), ImagePullSecrets:[]core.LocalObjectReference(nil), Hostname:"", Subdomain:"", SetHostnameAsFQDN:(*bool)(nil), Affinity:(*core.Affinity)(nil), SchedulerName:"default-scheduler", Tolerations:[]core.Toleration(nil), HostAliases:[]core.HostAlias(nil), PriorityClassName:"", Priority:(*int32)(nil), PreemptionPolicy:(*core.PreemptionPolicy)(nil), DNSConfig:(*core.PodDNSConfig)(nil), ReadinessGates:[]core.PodReadinessGate(nil), RuntimeClassName:(*string)(nil), Overhead:core.ResourceList(nil), EnableServiceLinks:(*bool)(nil), TopologySpreadConstraints:[]core.TopologySpreadConstraint(nil), OS:(*core.PodOS)(nil), SchedulingGates:[]core.PodSchedulingGate(nil), ResourceClaims:[]core.PodResourceClaim(nil)}}: field is immutable

in this case, the randomness is here: ...SecurityContext:(*core.PodSecurityContext)(0x400ae701b0)...

@turkenh
Copy link
Collaborator

turkenh commented Feb 8, 2024

@bobh66 do you still want to proceed here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants