Skip to content

Commit

Permalink
remove IsUnstructuredCRDReady
Browse files Browse the repository at this point in the history 
This commit removes `IsUnstructuredCRDReady` since
kubernetes/kubernetes#87675 is fixed.
Is uses `Is1CRDReady` to check the readiness of CRD.

After v1.7 we may consider merge the funcx `IsV1Beta1CRDReady` and
`IsV1CRDReady`

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
  • Loading branch information
@reasonerjt
reasonerjt committed Aug 31, 2021
1 parent 7c75cd6 commit a58469f
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 113 deletions.
7 changes: 1 addition & 6 deletions pkg/restore/restore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -842,12 +842,7 @@ func (ctx *restoreContext) crdAvailable(name string, crdClient client.Dynamic) (
if err != nil {
return true, err
}

// TODO: Due to upstream conversion issues in runtime.FromUnstructured,
// we use the unstructured object here. Once the upstream conversion
// functions are fixed, we should convert to the CRD types and use
// IsCRDReady.
available, err = kube.IsUnstructuredCRDReady(unstructuredCRD)
available, err = kube.IsCRDReady(unstructuredCRD)
if err != nil {
return true, err
}
Expand Down
77 changes: 21 additions & 56 deletions pkg/util/kube/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/wait"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
corev1listers "k8s.io/client-go/listers/core/v1"
Expand Down Expand Up @@ -176,61 +177,25 @@ func IsV1Beta1CRDReady(crd *apiextv1beta1.CustomResourceDefinition) bool {
return (isEstablished && namesAccepted)
}

// IsUnstructuredCRDReady checks an unstructured CRD to see if it's ready, with both the Established and NamesAccepted conditions.
// TODO: Delete this function and use IsV1CRDReady/IsV1Beta1CRDReady when the upstream runtime.FromUnstructured function properly handles int64 field conversions.
// Duplicated function because the velero install package uses IsV1CRDReady/IsV1Beta1CRDReady with instances of v1/v1beta1 types.
// See https://github.com/kubernetes/kubernetes/issues/87675
// This is different from the fix for https://github.com/vmware-tanzu/velero/issues/2319 because here,
// we need to account for *both* v1beta1 and v1 CRDs, so doing marshalling into JSON to convert to a Go type may not be as useful here, unless we do
// type switching.
func IsUnstructuredCRDReady(crd *unstructured.Unstructured) (bool, error) {
var isEstablished, namesAccepted bool

conditions, ok, err := unstructured.NestedSlice(crd.UnstructuredContent(), "status", "conditions")
if !ok {
return false, nil
}
if err != nil {
return false, errors.Wrap(err, "unable to access CRD's conditions")
}

for _, c := range conditions {
// Unlike the typed version of this function, we need to cast the Condition since it's an interface{} here,
// then we fetch the type and status of the Condition before inspecting them for relevant values
cond, ok := c.(map[string]interface{})
if !ok {
return false, errors.New("unable to convert condition to map[string]interface{}")
}
conditionType, ok, err := unstructured.NestedString(cond, "type")
if !ok {
// This should never happen unless someone manually edits the serialized data.
return false, errors.New("condition missing a type")
}

if err != nil {
return false, errors.Wrap(err, "unable to access condition's type")
}

status, ok, err := unstructured.NestedString(cond, "status")
if !ok {
// This should never happen unless someone manually edits the serialized data.
return false, errors.New("condition missing a status")
}

if err != nil {
return false, errors.Wrap(err, "unable to access condition's status")
}

// Here is the actual logic of the function
// Cast the API's types into strings since we're pulling strings out of the unstructured data.
// We are using the v1beta1 constants here but they are the same as the v1 constants.
if conditionType == string(apiextv1beta1.Established) && status == string(apiextv1beta1.ConditionTrue) {
isEstablished = true
}
if conditionType == string(apiextv1beta1.NamesAccepted) && status == string(apiextv1beta1.ConditionTrue) {
namesAccepted = true
}
// IsCRDReady triggers IsV1Beta1CRDReady/IsV1CRDReady according to the version of the input param
func IsCRDReady(crd *unstructured.Unstructured)(bool, error) {
ver := crd.GroupVersionKind().Version
switch ver {
case "v1beta1":
v1beta1crd := &apiextv1beta1.CustomResourceDefinition{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(crd.Object, v1beta1crd)
if err != nil{
return false, err
}
return IsV1Beta1CRDReady(v1beta1crd), nil
case "v1":
v1crd := &apiextv1.CustomResourceDefinition{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(crd.Object, v1crd)
if err != nil{
return false, err
}
return IsV1CRDReady(v1crd), nil
default:
return false, fmt.Errorf("unable to handle CRD with version %s", ver)
}

return (isEstablished && namesAccepted), nil
}
134 changes: 83 additions & 51 deletions pkg/util/kube/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package kube

import (
"encoding/json"
"testing"
"time"

Expand Down Expand Up @@ -288,31 +289,31 @@ func TestIsV1CRDReady(t *testing.T) {
}
}

func TestIsUnstructuredCRDReady(t *testing.T) {
tests := []struct {
func TestIsCRDReady(t *testing.T) {
v1beta1tests := []struct {
name string
crd *apiextv1beta1.CustomResourceDefinition
want bool
}{
{
name: "CRD is not established & not accepting names - not ready",
name: "v1beta1CRD is not established & not accepting names - not ready",
crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").Result(),
want: false,
},
{
name: "CRD is established & not accepting names - not ready",
name: "v1beta1CRD is established & not accepting names - not ready",
crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").
Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).Result(),
want: false,
},
{
name: "CRD is not established & accepting names - not ready",
name: "v1beta1CRD is not established & accepting names - not ready",
crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").
Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).Result(),
want: false,
},
{
name: "CRD is established & accepting names - ready",
name: "v1beta1CRD is established & accepting names - ready",
crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").
Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).
Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).
Expand All @@ -321,66 +322,97 @@ func TestIsUnstructuredCRDReady(t *testing.T) {
},
}

for _, tc := range tests {
for _, tc := range v1beta1tests {
m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.crd)
require.NoError(t, err)
result, err := IsUnstructuredCRDReady(&unstructured.Unstructured{Object: m})
result, err := IsCRDReady(&unstructured.Unstructured{Object: m})
require.NoError(t, err)
assert.Equal(t, tc.want, result)
}
}

// TestFromUnstructuredIntToFloatBug tests for a bug where runtime.DefaultUnstructuredConverter.FromUnstructured can't take a whole number into a float.
// This test should fail when https://github.com/kubernetes/kubernetes/issues/87675 is fixed upstream, letting us know we can remove the IsUnstructuredCRDReady function.
/*
func TestFromUnstructuredIntToFloatBug(t *testing.T) {
b := []byte(`
v1tests := []struct {
name string
crd *apiextv1.CustomResourceDefinition
want bool
}{
{
name: "v1CRD is not established & not accepting names - not ready",
crd: builder.ForV1CustomResourceDefinition("MyCRD").Result(),
want: false,
},
{
name: "v1CRD is established & not accepting names - not ready",
crd: builder.ForV1CustomResourceDefinition("MyCRD").
Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.Established).Status(apiextv1.ConditionTrue).Result()).Result(),
want: false,
},
{
name: "v1CRD is not established & accepting names - not ready",
crd: builder.ForV1CustomResourceDefinition("MyCRD").
Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NamesAccepted).Status(apiextv1.ConditionTrue).Result()).Result(),
want: false,
},
{
name: "v1CRD is established & accepting names - ready",
crd: builder.ForV1CustomResourceDefinition("MyCRD").
Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.Established).Status(apiextv1.ConditionTrue).Result()).
Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NamesAccepted).Status(apiextv1.ConditionTrue).Result()).
Result(),
want: true,
},
}

for _, tc := range v1tests {
m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.crd)
require.NoError(t, err)
result, err := IsCRDReady(&unstructured.Unstructured{Object: m})
require.NoError(t, err)
assert.Equal(t, tc.want, result)
}

// input param is unrecognized
resBytes := []byte(`
{
"apiVersion": "apiextensions.k8s.io/v1beta1",
"apiVersion": "apiextensions.k8s.io/v9",
"kind": "CustomResourceDefinition",
"metadata": {
"name": "foos.example.foo.com"
"name": "foos.example.foo.com"
},
"spec": {
"group": "example.foo.com",
"version": "v1alpha1",
"scope": "Namespaced",
"names": {
"plural": "foos",
"singular": "foo",
"kind": "Foo"
},
"validation": {
"openAPIV3Schema": {
"required": [
"spec"
],
"properties": {
"spec": {
"required": [
"bar"
],
"properties": {
"bar": {
"type": "integer",
"minimum": 1
"group": "example.foo.com",
"version": "v1alpha1",
"scope": "Namespaced",
"names": {
"plural": "foos",
"singular": "foo",
"kind": "Foo"
},
"validation": {
"openAPIV3Schema": {
"required": [
"spec"
],
"properties": {
"spec": {
"required": [
"bar"
],
"properties": {
"bar": {
"type": "integer",
"minimum": 1
}
}
}
}
}
}
}
}
}
}
}
}
`)
var obj unstructured.Unstructured
err := json.Unmarshal(b, &obj)
obj := &unstructured.Unstructured{}
err := json.Unmarshal(resBytes, obj)
require.NoError(t, err)
var newCRD apiextv1beta1.CustomResourceDefinition
err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &newCRD)
// If there's no error, then the upstream issue is fixed, and we need to remove our workarounds.
require.Error(t, err)
_, err = IsCRDReady(obj)
assert.NotNil(t, err)
}
*/

0 comments on commit a58469f

Please sign in to comment.