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

Remove EnqueueRequestForAnnotation handler from SDK #3506

Merged
merged 1 commit into from
Jul 24, 2020
Merged

Remove EnqueueRequestForAnnotation handler from SDK #3506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions changelog/fragments/mv-EnqueueRequestForAnnotation.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
entries:
- description: >
Remove the implementation for `EnqueueRequestForAnnotation` handler from sdk repository and
reference it from operator-lib instead.
kind: "change"

# Is this a breaking change?
breaking: true

# Migration can be defined to automatically add a section to
# the migration guide. This is required for breaking changes.
migration:
header: Move the implementation of `EnqueueRequestForAnnotation` handler
body: >
The implementation of `EnqueueRequestForAnnotation` handler has been removed from Operator SDK repository
and is now imported from Operator-lib.
18 changes: 9 additions & 9 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ require (
github.com/markbates/inflect v1.0.4
github.com/mattn/go-isatty v0.0.12
github.com/mitchellh/go-homedir v1.1.0
github.com/onsi/ginkgo v1.12.0
github.com/onsi/gomega v1.9.0
github.com/onsi/ginkgo v1.12.1
github.com/onsi/gomega v1.10.1
github.com/operator-framework/api v0.3.8
github.com/operator-framework/operator-lib v0.0.0-20200722145423-0ffd6fc49593
github.com/operator-framework/operator-lib v0.0.0-20200723212032-525cb9a9ed70
github.com/operator-framework/operator-registry v1.12.6-0.20200611222234-275301b779f8
github.com/prometheus/client_golang v1.5.1
github.com/rogpeppe/go-internal v1.5.0
Expand All @@ -30,18 +30,18 @@ require (
go.uber.org/zap v1.14.1
golang.org/x/tools v0.0.0-20200403190813-44a64ad78b9b
gomodules.xyz/jsonpatch/v3 v3.0.1
gopkg.in/yaml.v2 v2.2.8
gopkg.in/yaml.v2 v2.3.0
gopkg.in/yaml.v3 v3.0.0-20190905181640-827449938966
helm.sh/helm/v3 v3.2.4
k8s.io/api v0.18.2
k8s.io/apiextensions-apiserver v0.18.2
k8s.io/apimachinery v0.18.2
k8s.io/api v0.18.4
k8s.io/apiextensions-apiserver v0.18.4
k8s.io/apimachinery v0.18.4
k8s.io/cli-runtime v0.18.2
k8s.io/client-go v0.18.2
k8s.io/client-go v0.18.4
k8s.io/klog v1.0.0
k8s.io/kubectl v0.18.2
rsc.io/letsencrypt v0.0.3 // indirect
sigs.k8s.io/controller-runtime v0.6.0
sigs.k8s.io/controller-runtime v0.6.1
sigs.k8s.io/controller-tools v0.3.0
sigs.k8s.io/kubebuilder v1.0.9-0.20200618125005-36aa113dbe99
sigs.k8s.io/yaml v1.2.0
Expand Down
64 changes: 62 additions & 2 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 4 additions & 12 deletions internal/generate/crd/crd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,10 @@ spec:
description: Memcached is the Schema for the memcacheds API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
Copy link
Member Author

@varshaprasad96 varshaprasad96 Jul 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The newlines present in-between strings were not passing the tests. So removed them.

Copy link
Member

Choose a reason for hiding this comment

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

They pass in master.

Copy link
Contributor

@camilamacedo86 camilamacedo86 Jul 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

What was the error? Can you share?

Copy link
Member Author

@varshaprasad96 varshaprasad96 Jul 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

There were errors due to the presence of newline in the expected string output with backticks (back quotes). This was after bumping versions of some of the dependencies along with controller-runtime (though I didn't look into what was the exact one causing this). This does not happen in master. For ex in case of one of the test TestCRDGo:

Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -19,10 +19,6 @@
            	            	         apiVersion:
            	            	-		  description: 'APIVersion defines the versioned schema of this representation
            	            	-		  of an object. Servers should convert recognized schemas to the latest 
            	            	-		  internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
            	            	+          description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
            	            	           type: string
            	            	         kind:
            	            	-		  description: 'Kind is a string value representing the REST resource this 
            	            	-		  object represents. Servers may infer this from the endpoint the client 
            	            	-		  submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
            	            	+          description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
            	            	           type: string

Copy link
Member

Choose a reason for hiding this comment

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

@varshaprasad96 I saw the same error in my new PR #3523. I think the new version of ginkgo or gomega caused it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh..The failing tests here are written in the old golang testing framework though. Not sure which, but some other dependency maybe causing it.

type: string
metadata:
type: object
Expand Down Expand Up @@ -361,14 +357,10 @@ spec:
description: Memcached is the Schema for the memcacheds API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
Expand Down
4 changes: 2 additions & 2 deletions pkg/ansible/proxy/cache_response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import (
"regexp"
"strings"

libhandler "github.com/operator-framework/operator-lib/handler"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/requestfactory"
k8sRequest "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/requestfactory"
osdkHandler "github.com/operator-framework/operator-sdk/pkg/handler"

"k8s.io/apimachinery/pkg/api/meta"
metainternalscheme "k8s.io/apimachinery/pkg/apis/meta/internalversion/scheme"
Expand Down Expand Up @@ -224,7 +224,7 @@ func (c *cacheResponseHandler) recoverDependentWatches(req *http.Request, un *un
}
}
}
if typeString, ok := un.GetAnnotations()[osdkHandler.TypeAnnotation]; ok {
if typeString, ok := un.GetAnnotations()[libhandler.TypeAnnotation]; ok {
ownerGV, err := schema.ParseGroupVersion(ownerRef.APIVersion)
if err != nil {
m := fmt.Sprintf("could not get group version for: %v", ownerGV)
Expand Down
10 changes: 8 additions & 2 deletions pkg/ansible/proxy/inject_owner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
"net/http"
"net/http/httputil"

"github.com/operator-framework/operator-lib/handler"
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap"
k8sRequest "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/requestfactory"
"github.com/operator-framework/operator-sdk/pkg/handler"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -146,7 +146,13 @@ func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http
if addOwnerRef {
data.SetOwnerReferences(append(data.GetOwnerReferences(), owner.OwnerReference))
} else {
handler.SetOwnerAnnotation(data, ownerObject)
err := handler.SetOwnerAnnotations(data, ownerObject)
if err != nil {
m := "Could not set owner annotations"
log.Error(err, m)
http.Error(w, m, http.StatusBadRequest)
return
}
}
newBody, err := json.Marshal(data.Object)
if err != nil {
Expand Down
20 changes: 11 additions & 9 deletions pkg/ansible/proxy/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import (
"sync"
"time"

libhandler "github.com/operator-framework/operator-lib/handler"
"github.com/operator-framework/operator-lib/predicate"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/kubeconfig"
k8sRequest "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/requestfactory"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -38,12 +43,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/source"

"github.com/operator-framework/operator-lib/predicate"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/controllermap"
"github.com/operator-framework/operator-sdk/pkg/ansible/proxy/kubeconfig"
k8sRequest "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/requestfactory"
osdkHandler "github.com/operator-framework/operator-sdk/pkg/handler"
)

// This is the default timeout to wait for the cache to respond
Expand Down Expand Up @@ -257,11 +256,14 @@ func addWatchToController(owner kubeconfig.NamespacedOwnerReference, cMap *contr
return nil
}
awMap.Store(resource.GroupVersionKind())
typeString := fmt.Sprintf("%v.%v", owner.Kind, ownerGV.Group)
ownerGK := schema.GroupKind{
Kind: owner.Kind,
Group: ownerGV.Group,
}
log.Info("Watching child resource", "kind", resource.GroupVersionKind(),
"enqueue_annotation_type", typeString)
"enqueue_annotation_type", ownerGK.String())
err = contents.Controller.Watch(&source.Kind{Type: resource},
&osdkHandler.EnqueueRequestForAnnotation{Type: typeString}, predicate.DependentPredicate{})
&libhandler.EnqueueRequestForAnnotation{Type: ownerGK}, predicate.DependentPredicate{})
if err != nil {
log.Error(err, "Failed to watch child resource",
"kind", resource.GroupVersionKind(), "enqueue_kind", u.GroupVersionKind())
Expand Down
127 changes: 0 additions & 127 deletions pkg/handler/enqueue_annotation.go
View file
Open in desktop

This file was deleted.

7 changes: 5 additions & 2 deletions pkg/helm/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"errors"
"io"

"github.com/operator-framework/operator-lib/handler"
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
"github.com/operator-framework/operator-sdk/pkg/handler"
"helm.sh/helm/v3/pkg/kube"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -148,7 +148,10 @@ func (c *ownerRefInjectingClient) Build(reader io.Reader, validate bool) (kube.R
ownerRef := metav1.NewControllerRef(c.owner, c.owner.GroupVersionKind())
u.SetOwnerReferences([]metav1.OwnerReference{*ownerRef})
} else {
handler.SetOwnerAnnotation(u, c.owner)
err := handler.SetOwnerAnnotations(u, c.owner)
if err != nil {
return err
}
}
return nil
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/helm/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"
"sigs.k8s.io/yaml"

libhandler "github.com/operator-framework/operator-lib/handler"
"github.com/operator-framework/operator-lib/predicate"
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
"github.com/operator-framework/operator-sdk/pkg/handler"
Expand Down Expand Up @@ -132,7 +133,7 @@ func watchDependentResources(mgr manager.Manager, r *HelmOperatorReconciler, c c
return err
}
} else { // Setup watch using annotations.
err = c.Watch(&source.Kind{Type: &u}, &handler.EnqueueRequestForAnnotation{Type: gvk.GroupKind().String()},
err = c.Watch(&source.Kind{Type: &u}, &libhandler.EnqueueRequestForAnnotation{Type: gvk.GroupKind()},
predicate.DependentPredicate{})
if err != nil {
return err
Expand Down