Repository: istio/istio
File: cni/pkg/plugin/plugin.go (Go)

Code before:
	// Pass through the result for the next plugin
	return types.PrintResult(conf.PrevResult, conf.CNIVersion)
}

func CmdCheck(args *skel.CmdArgs) (err error) {
	return nil
}

func CmdDelete(args *skel.CmdArgs) (err error) {
	return nil
}

func isAmbientPod(client kubernetes.Interface, podName, podNamespace string, selectors []util.EnablementSelector) (bool, error) {
	compiledSelectors, err := util.NewCompiledEnablementSelectors(selectors)
	if err != nil {
		return false, fmt.Errorf("failed to instantiate ambient enablement selector: %v", err)
	}

	var pod *v1.Pod
	var ns *v1.Namespace
	var podErr, nsErr error
	for attempt := 1; attempt <= podRetrievalMaxRetries; attempt++ {
		if pod == nil || podErr != nil {
			pod, podErr = client.CoreV1().Pods(podNamespace).Get(context.Background(), podName, metav1.GetOptions{})
		}
		if ns == nil || nsErr != nil {
			ns, nsErr = client.CoreV1().Namespaces().Get(context.Background(), podNamespace, metav1.GetOptions{})
		}
		if podErr != nil || nsErr != nil {
			log.Debugf("failed to get pod or namespace info, retrying: podErr=%v, nsErr=%v", podErr, nsErr)
			time.Sleep(podRetrievalInterval)
			continue
		}
	}
	if podErr != nil || nsErr != nil || pod == nil || ns == nil {
		return false, fmt.Errorf("failed to get pod or namespace info: podErr=%v, nsErr=%v", podErr, nsErr)
	}

	return compiledSelectors.Matches(pod.Labels, pod.Annotations, ns.Labels), nil
}

func isCNIPod(conf *Config, k8sArgs *K8sArgs) bool {
	if strings.HasPrefix(string(k8sArgs.K8S_POD_NAME), "istio-cni-node-") &&
		string(k8sArgs.K8S_POD_NAMESPACE) == conf.PodNamespace {
		log.Infof("in a degraded state and %v looks like our own agent pod, skipping", k8sArgs.K8S_POD_NAME)
		return true
	}
	log.Warnf("not a CNI pod, podName: %s, podNamespace: %s, conf pod namespace: %s", k8sArgs.K8S_POD_NAME, k8sArgs.K8S_POD_NAMESPACE, conf.PodNamespace)
	return false
}


Review comment (bug):
If we stop retrying for an object once it’s retrieved, we could end up with stale data for the other object. For example:
- Suppose we successfully retrieve the namespace object first.
- There’s a delay in retrieving the pod object due to API server errors.
- During this delay, the namespace (previously not part of the mesh) gets added to the mesh.
- Because we’re still retrying for the pod object, the CNI might assume the pod is not ambient-enabled due to the stale namespace object lacking the required label.

This might be mitigated by event handlers, so it may not be a major issue. However, there could still be edge cases where stale data causes inconsistencies. Let me know your thoughts on this.

Code after:
	if conf.PrevResult == nil {
		result = &cniv1.Result{
			CNIVersion: cniv1.ImplementedSpecVersion,
		}
		return types.PrintResult(result, conf.CNIVersion)
	}

	// Pass through the result for the next plugin
	return types.PrintResult(conf.PrevResult, conf.CNIVersion)
}

func CmdCheck(args *skel.CmdArgs) (err error) {
	return nil
}

func CmdDelete(args *skel.CmdArgs) (err error) {
	return nil
}

func isAmbientPod(client kubernetes.Interface, podName, podNamespace string, selectors []util.EnablementSelector, enableRetry bool) (bool, error) {
	compiledSelectors, err := util.NewCompiledEnablementSelectors(selectors)
	if err != nil {
		return false, fmt.Errorf("failed to instantiate ambient enablement selector: %v", err)
	}

	maxRetries := 1
	if enableRetry {
		maxRetries = podRetrievalMaxRetries
	}

	var pod *v1.Pod
	var ns *v1.Namespace
	var podErr, nsErr error

	err = backoff.Retry(func() error {
		// attempt to get pod and namespace
		pod, podErr = client.CoreV1().Pods(podNamespace).Get(context.Background(), podName, metav1.GetOptions{})
		ns, nsErr = client.CoreV1().Namespaces().Get(context.Background(), podNamespace, metav1.GetOptions{})
		if podErr != nil || nsErr != nil {
			errMsg := fmt.Sprintf("failed to get pod or namespace info, retrying: podErr=%v, nsErr=%v", podErr, nsErr)
			log.Debug(errMsg)
			// reset pod and ns to force re-get to avoid stale resources
			pod, ns = nil, nil
			return errors.New(errMsg)
		}
		return nil
	}, backoff.WithMaxRetries(backoff.NewConstantBackOff(podRetrievalInterval), uint64(maxRetries)))
	if err != nil {
		return false, fmt.Errorf("failed to get pod or namespace info: podErr=%v, nsErr=%v", podErr, nsErr)
	}