Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Attempting to enable federation of an already-enabled type should fail fast #743

Closed
marun opened this issue Apr 10, 2019 · 4 comments · Fixed by #802
Closed

Attempting to enable federation of an already-enabled type should fail fast #743

marun opened this issue Apr 10, 2019 · 4 comments · Fixed by #802
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@marun
Copy link
Contributor

marun commented Apr 10, 2019

Attempting to enable federation of an already-enabled type with a non-default group will create the crd for the federated type and then fail in attempting to create the FederatedTypeConfig (since it already exists). kubefed2 enable should check for the presence of the FTC for the type and fail immediately if it already exists, since a type can be enabled at most once for a control plane.

/kind bug

@marun marun added this to the v1beta1 milestone Apr 10, 2019
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Apr 10, 2019
@draveness
Copy link
Contributor

draveness commented Apr 22, 2019

Is this issue still exists? I checked out the code in enable.go file and found out the cmd will update the FederatedTypeConfig if it already exists.

https://github.com/kubernetes-sigs/federation-v2/blob/master/pkg/kubefed2/enable/enable.go#L275-L297

	concreteTypeConfig := resources.TypeConfig.(*fedv1a1.FederatedTypeConfig)
	concreteTypeConfig.Namespace = namespace
	existingTypeConfig := &fedv1a1.FederatedTypeConfig{}
	err = client.Get(context.TODO(), existingTypeConfig, namespace, concreteTypeConfig.Name)
	createdOrUpdated := "created"
	if err != nil {
		if !apierrors.IsNotFound(err) {
			return errors.Wrapf(err, "Error retrieving FederatedTypeConfig %q", concreteTypeConfig.Name)
		}
		err = client.Create(context.TODO(), concreteTypeConfig)
		if err != nil {
			return errors.Wrapf(err, "Error creating FederatedTypeConfig %q", concreteTypeConfig.Name)
		}
	} else {
		existingTypeConfig.Spec = concreteTypeConfig.Spec
		err = client.Update(context.TODO(), existingTypeConfig)
		if err != nil {
			return errors.Wrapf(err, "Error updating FederatedTypeConfig %q", concreteTypeConfig.Name)
		}
		createdOrUpdated = "updated"
	}

@font
Copy link
Contributor

font commented Apr 22, 2019

This issue has been superseded by #720. I would close this but considering this issue was created after #720, I'll defer to @marun.

@marun
Copy link
Contributor Author

marun commented Apr 24, 2019

@font This issue was not been superseded by #720. If you inspect the code, you will notice that CRD creation is performed before checking if an FTC exists. This allows creation of a duplicate CRD with non-default group/version. A check for the FTC should instead be performed in advance of CRD creation, and enabling should fail if the group and version of the CRD that will be created don't match an existing FTC.

@font
Copy link
Contributor

font commented Apr 25, 2019

@marun Apologies I misread your original comment. The important keyword was non-default group.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants