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.

Sign up for GitHub

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

Bug 1746270: Remove deprecated CRD's stored versions to allow CRD update #983

Merged
merged 2 commits into from
Sep 10, 2019
Merged

Bug 1746270: Remove deprecated CRD's stored versions to allow CRD update #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b873c79
Remove deprecated CRD's stored versions to allow CRD update
dinhxuanvu Aug 15, 2019
4bb8526
Add unit and e2e test cases for deprecating a CRD version
dinhxuanvu Aug 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 56 additions & 17 deletions pkg/controller/operators/catalog/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1128,17 +1128,23 @@ func (o *Operator) ResolvePlan(plan *v1alpha1.InstallPlan) error {
return nil
}

// Ensure all existing served versions are present in new CRD
func ensureCRDVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) error {
newCRDVersions := map[string]struct{}{}
func getCRDVersionsMap(crd *v1beta1ext.CustomResourceDefinition) map[string]struct{} {
versionsMap := map[string]struct{}{}

for _, newVersion := range newCRD.Spec.Versions {
newCRDVersions[newVersion.Name] = struct{}{}
for _, version := range crd.Spec.Versions {
versionsMap[version.Name] = struct{}{}
}
if newCRD.Spec.Version != "" {
newCRDVersions[newCRD.Spec.Version] = struct{}{}
if crd.Spec.Version != "" {
versionsMap[crd.Spec.Version] = struct{}{}
}

return versionsMap
}

// Ensure all existing served versions are present in new CRD
func ensureCRDVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) error {
newCRDVersions := getCRDVersionsMap(newCRD)

for _, oldVersion := range oldCRD.Spec.Versions {
if oldVersion.Served {
_, ok := newCRDVersions[oldVersion.Name]
Expand Down Expand Up @@ -1203,10 +1209,36 @@ func (o *Operator) validateExistingCRs(gvr schema.GroupVersionResource, newCRD *
return fmt.Errorf("error validating custom resource against new schema %#v: %s", newCRD.Spec.Validation, err)
}
}

return nil
}

// Attempt to remove stored versions that have been deprecated before allowing
// those versions to be removed from the new CRD.
// The function may not always succeed as storedVersions requires at least one
// version. If there is only stored version, it won't be removed until a new
// stored version is added.
func removeDeprecatedStoredVersions(oldCRD *v1beta1ext.CustomResourceDefinition, newCRD *v1beta1ext.CustomResourceDefinition) []string {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice change

// StoredVersions requires to have at least one version.
if len(oldCRD.Status.StoredVersions) <= 1 {
return nil
}

newStoredVersions := []string{}
newCRDVersions := getCRDVersionsMap(newCRD)
for _, v := range oldCRD.Status.StoredVersions {
_, ok := newCRDVersions[v]
if ok {
newStoredVersions = append(newStoredVersions, v)
}
}

if len(newStoredVersions) < 1 {
return nil
} else {
return newStoredVersions
}
}

// ExecutePlan applies a planned InstallPlan to a namespace.
func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
if plan.Status.Phase != v1alpha1.InstallPlanPhaseInstalling {
Expand Down Expand Up @@ -1256,7 +1288,9 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
if k8serrors.IsAlreadyExists(err) {
currentCRD, _ := o.lister.APIExtensionsV1beta1().CustomResourceDefinitionLister().Get(crd.GetName())
// Compare 2 CRDs to see if it needs to be updatetd
if !reflect.DeepEqual(crd, *currentCRD) {
if !(reflect.DeepEqual(crd.Spec.Version, currentCRD.Spec.Version) &&
reflect.DeepEqual(crd.Spec.Versions, currentCRD.Spec.Versions) &&
reflect.DeepEqual(crd.Spec.Validation, currentCRD.Spec.Validation)) {
// Verify CRD ownership, only attempt to update if
// CRD has only one owner
// Example: provided=database.coreos.com/v1alpha1/EtcdCluster
Expand All @@ -1267,11 +1301,6 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
crd.SetResourceVersion(currentCRD.GetResourceVersion())
if len(matchedCSV) == 1 {
o.logger.Debugf("Found one owner for CRD %v", crd)

_, err = o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd)
if err != nil {
return errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
}
} else if len(matchedCSV) > 1 {
o.logger.Debugf("Found multiple owners for CRD %v", crd)

Expand All @@ -1283,11 +1312,21 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
if err = o.validateCustomResourceDefinition(currentCRD, &crd); err != nil {
return errorwrap.Wrapf(err, "error validating existing CRs agains new CRD's schema: %s", step.Resource.Name)
}

_, err = o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd)
}
// Remove deprecated version in CRD storedVersions
storeVersions := removeDeprecatedStoredVersions(currentCRD, &crd)
if storeVersions != nil {
currentCRD.Status.StoredVersions = storeVersions
resultCRD, err := o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().UpdateStatus(currentCRD)
if err != nil {
return errorwrap.Wrapf(err, "error update CRD: %s", step.Resource.Name)
return errorwrap.Wrapf(err, "error updating CRD's status: %s", step.Resource.Name)
}
crd.SetResourceVersion(resultCRD.GetResourceVersion())
}
// Update CRD to new version
_, err = o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd)
if err != nil {
return errorwrap.Wrapf(err, "error updating CRD: %s", step.Resource.Name)
}
}
// If it already existed, mark the step as Present.
Expand Down
99 changes: 99 additions & 0 deletions pkg/controller/operators/catalog/operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,105 @@ func TestEnsureCRDVersions(t *testing.T) {
}
}

func TestRemoveDeprecatedStoredVersions(t *testing.T) {
mainCRDPlural := "ins-main-test"

currentVersions := []v1beta1.CustomResourceDefinitionVersion{
{
Name: "v1alpha1",
Served: true,
Storage: false,
},
{
Name: "v1alpha2",
Served: true,
Storage: true,
},
}

newVersions := []v1beta1.CustomResourceDefinitionVersion{
{
Name: "v1alpha2",
Served: true,
Storage: false,
},
{
Name: "v1beta1",
Served: true,
Storage: true,
},
}

crdStatusStoredVersions := v1beta1.CustomResourceDefinitionStatus{
StoredVersions: []string{},
}

tests := []struct {
name string
oldCRD v1beta1.CustomResourceDefinition
newCRD v1beta1.CustomResourceDefinition
expectedResult []string
}{
{
name: "only one stored version exists",
oldCRD: func() v1beta1.CustomResourceDefinition {
oldCRD := crd(mainCRDPlural)
oldCRD.Spec.Version = ""
oldCRD.Spec.Versions = currentVersions
oldCRD.Status = crdStatusStoredVersions
oldCRD.Status.StoredVersions = []string{"v1alpha1"}
return oldCRD
}(),
newCRD: func() v1beta1.CustomResourceDefinition {
newCRD := crd(mainCRDPlural)
newCRD.Spec.Version = ""
newCRD.Spec.Versions = newVersions
return newCRD
}(),
expectedResult: nil,
},
{
name: "multiple stored versions with one deprecated version",
oldCRD: func() v1beta1.CustomResourceDefinition {
oldCRD := crd(mainCRDPlural)
oldCRD.Spec.Version = ""
oldCRD.Spec.Versions = currentVersions
oldCRD.Status.StoredVersions = []string{"v1alpha1", "v1alpha2"}
return oldCRD
}(),
newCRD: func() v1beta1.CustomResourceDefinition {
newCRD := crd(mainCRDPlural)
newCRD.Spec.Version = ""
newCRD.Spec.Versions = newVersions
return newCRD
}(),
expectedResult: []string{"v1alpha2"},
},
{
name: "multiple stored versions with all deprecated version",
oldCRD: func() v1beta1.CustomResourceDefinition {
oldCRD := crd(mainCRDPlural)
oldCRD.Spec.Version = ""
oldCRD.Spec.Versions = currentVersions
oldCRD.Status.StoredVersions = []string{"v1alpha1", "v1alpha3"}
return oldCRD
}(),
newCRD: func() v1beta1.CustomResourceDefinition {
newCRD := crd(mainCRDPlural)
newCRD.Spec.Version = ""
newCRD.Spec.Versions = newVersions
return newCRD
}(),
expectedResult: nil,
},
}

for _, tt := range tests {
resultCRD := removeDeprecatedStoredVersions(&tt.oldCRD, &tt.newCRD)
require.Equal(t, tt.expectedResult, resultCRD)
}
}

func TestExecutePlan(t *testing.T) {
namespace := "ns"

Expand Down
Loading