Repository: k3s-io/k3s
File: pkg/deploy/controller.go (Go)

Code before:
		}
	}

	content, err := os.ReadFile(path)
	if err != nil {
		w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ReadManifestFailed", "Read manifest at %q failed: %v", path, err)
	} else {
		if o, err := objectSet(content); err != nil {
			w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ParseManifestFailed", "Parse manifest at %q failed: %v", path, err)
		} else {
			// Search for objects using both GVKs currently listed in the file, as well as GVKs previously applied.
			// This ensures that any conflicts between competing deploy controllers are handled properly.
			addonGVKs = append(addonGVKs, o.GVKs()...)
		}
	}

	// Ensure that we don't try to delete using GVKs that the server doesn't have.
	// This can happen when CRDs are removed or when core types are removed - PodSecurityPolicy, for example.
	addonGVKs, err = w.validateGVKs(addonGVKs)
	if err != nil {
		return err
	}

	// ensure that the addon is completely removed before deleting the objectSet
	w.recorder.Eventf(&addon, corev1.EventTypeNormal, "DeletingManifest", "Deleting manifest at %q", path)
	if err := w.addons.Delete(addon.Namespace, addon.Name, &metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
		return err
	}

	// apply an empty set with owner & gvk data to delete
	if err := w.apply.WithOwner(&addon).WithGVK(addonGVKs...).ApplyObjects(); err != nil {
		return err
	}

	return os.Remove(path)
}

// getOrCreateAddon attempts to get an Addon by name from the addon namespace, and creates a new one
// if it cannot be found.
func (w *watcher) getOrCreateAddon(name string) (apisv1.Addon, error) {
	addon, err := w.addonCache.Get(metav1.NamespaceSystem, name)
	if apierrors.IsNotFound(err) {
		addon = apisv1.NewAddon(metav1.NamespaceSystem, name, apisv1.Addon{})
	} else if err != nil {
		return apisv1.Addon{}, err
	}
	return *addon, nil
}

// validateGVKs removes from the list any GVKs that the server does not support
func (w *watcher) validateGVKs(addonGVKs []schema.GroupVersionKind) ([]schema.GroupVersionKind, error) {

Review comment (refactor):
I am a little concerned about this, but only because the Wrangler OnRemove handler can behave in unexpected ways.

The fact that we were returning early here if the delete succeeds OR returns an error other than not-found makes me suspect that we are relying on the controller enqueueing and re-executing this handler multiple times. Otherwise the apply and file delete down below would never happen.

A lot has changed since this code and the comment associated with it were added. For example, we did not actually track GVKs at the time. I would almost suggest that in addition to this change, we move this down below to not remove the AddOn until AFTER the apply and file delete have succeeded. 

Code after:
		}
	}

	content, err := os.ReadFile(path)
	if err != nil {
		w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ReadManifestFailed", "Read manifest at %q failed: %v", path, err)
	} else {
		if o, err := objectSet(content); err != nil {
			w.recorder.Eventf(&addon, corev1.EventTypeWarning, "ParseManifestFailed", "Parse manifest at %q failed: %v", path, err)
		} else {
			// Search for objects using both GVKs currently listed in the file, as well as GVKs previously applied.
			// This ensures that any conflicts between competing deploy controllers are handled properly.
			addonGVKs = append(addonGVKs, o.GVKs()...)
		}
	}

	// Ensure that we don't try to delete using GVKs that the server doesn't have.
	// This can happen when CRDs are removed or when core types are removed - PodSecurityPolicy, for example.
	addonGVKs, err = w.validateGVKs(addonGVKs)
	if err != nil {
		return err
	}

	// Apply an empty set with owner & gvk data to delete
	if err := w.apply.WithOwner(&addon).WithGVK(addonGVKs...).ApplyObjects(); err != nil {
		return err
	}

	// Remove the addon file
	if err := os.Remove(path); err != nil {
		return err
	}

	// Delete the addon
	w.recorder.Eventf(&addon, corev1.EventTypeNormal, "DeletingManifest", "Deleting manifest at %q", path)
	if err := w.addons.Delete(addon.Namespace, addon.Name, &metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) {
		return err
	}

	return nil
}

// getOrCreateAddon attempts to get an Addon by name from the addon namespace, and creates a new one
// if it cannot be found.
func (w *watcher) getOrCreateAddon(name string) (apisv1.Addon, error) {
	addon, err := w.addonCache.Get(metav1.NamespaceSystem, name)
	if apierrors.IsNotFound(err) {
		addon = apisv1.NewAddon(metav1.NamespaceSystem, name, apisv1.Addon{})
	} else if err != nil {
		return apisv1.Addon{}, err
	}