Skip to content

Commit

Permalink
Merge pull request #18147 from DirectXMan12/bug/make-idler-use-owner-…
Browse files Browse the repository at this point in the history 
…refs

Automatic merge from submit-queue (batch tested with PRs 18108, 18150, 18164, 18147, 18143).

Switch idling over to using owner references.

This commit switches the `oc idle` code over to using owner references
instead of the created-by annotation and the OpenShift-specific
deployment config annotations.

Closes #17798
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Jan 19, 2018
2 parents 69bc601 + 4377932 commit 1533d3e
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 137 deletions.
81 changes: 21 additions & 60 deletions pkg/oc/cli/cmd/idle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import (
"k8s.io/kubernetes/pkg/kubectl/resource"

appsv1client "github.com/openshift/client-go/apps/clientset/versioned/typed/apps/v1"
"github.com/openshift/origin/pkg/api"
appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
appsmanualclient "github.com/openshift/origin/pkg/apps/client/v1"
"github.com/openshift/origin/pkg/oc/cli/util/clientcmd"
unidlingapi "github.com/openshift/origin/pkg/unidling/api"
Expand All @@ -47,7 +45,7 @@ var (

idleExample = templates.Examples(`
# Idle the scalable controllers associated with the services listed in to-idle.txt
$ %[1]s idle --resource-names-file to-idle.txt`)
$ %[1]s idle --resource-names-file to-idle.txt`)
)

// NewCmdIdle implements the OpenShift cli idle command
Expand Down Expand Up @@ -214,9 +212,9 @@ func (o *IdleOptions) calculateIdlableAnnotationsByService(f *clientcmd.Factory)
return pod, nil
}

controllersLoaded := make(map[kapi.ObjectReference]runtime.Object)
controllersLoaded := make(map[metav1.OwnerReference]metav1.Object)
helpers := make(map[schema.GroupKind]*resource.Helper)
getController := func(ref kapi.ObjectReference) (runtime.Object, error) {
getController := func(namespace string, ref metav1.OwnerReference) (metav1.Object, error) {
if controller, ok := controllersLoaded[ref]; ok {
return controller, nil
}
Expand All @@ -243,20 +241,24 @@ func (o *IdleOptions) calculateIdlableAnnotationsByService(f *clientcmd.Factory)
}

var controller runtime.Object
controller, err = helper.Get(ref.Namespace, ref.Name, false)
controller, err = helper.Get(namespace, ref.Name, false)
if err != nil {
return nil, err
}

controllersLoaded[ref] = controller
controllerMeta, err := meta.Accessor(controller)
if err != nil {
return nil, err
}

return controller, nil
controllersLoaded[ref] = controllerMeta

return controllerMeta, nil
}

targetScaleRefs := make(map[unidlingapi.CrossGroupObjectReference]types.NamespacedName)
endpointsInfo := make(map[types.NamespacedName]idleUpdateInfo)

decoder := f.Decoder(true)
err = o.svcBuilder.Do().Visit(func(info *resource.Info, err error) error {
if err != nil {
return err
Expand All @@ -271,7 +273,7 @@ func (o *IdleOptions) calculateIdlableAnnotationsByService(f *clientcmd.Factory)
Namespace: endpoints.Namespace,
Name: endpoints.Name,
}
scaleRefs, err := findScalableResourcesForEndpoints(endpoints, decoder, getPod, getController)
scaleRefs, err := findScalableResourcesForEndpoints(endpoints, getPod, getController)
if err != nil {
return fmt.Errorf("unable to calculate scalable resources for service %s/%s: %v", endpoints.Namespace, endpoints.Name, err)
}
Expand All @@ -293,41 +295,7 @@ func (o *IdleOptions) calculateIdlableAnnotationsByService(f *clientcmd.Factory)
return endpointsInfo, targetScaleRefs, err
}

// getControllerRef returns a subresource reference to the owning controller of the given object.
// It will use both the CreatedByAnnotation from Kubernetes, as well as the DeploymentConfigAnnotation
// from Origin to look this up. If neither are found, it will return nil.
func getControllerRef(obj runtime.Object, decoder runtime.Decoder) (*kapi.ObjectReference, error) {
objMeta, err := meta.Accessor(obj)
if err != nil {
return nil, err
}

annotations := objMeta.GetAnnotations()

creatorRefRaw, creatorListed := annotations[api.DeprecatedKubeCreatedByAnnotation]
if !creatorListed {
// if we don't have a creator listed, try the openshift-specific Deployment annotation
dcName, dcNameListed := annotations[appsapi.DeploymentConfigAnnotation]
if !dcNameListed {
return nil, nil
}

return &kapi.ObjectReference{
Name: dcName,
Namespace: objMeta.GetNamespace(),
Kind: "DeploymentConfig",
}, nil
}

serializedRef := &kapi.SerializedReference{}
if err := runtime.DecodeInto(decoder, []byte(creatorRefRaw), serializedRef); err != nil {
return nil, fmt.Errorf("could not decoded pod's creator reference: %v", err)
}

return &serializedRef.Reference, nil
}

func makeCrossGroupObjRef(ref *kapi.ObjectReference) (unidlingapi.CrossGroupObjectReference, error) {
func makeCrossGroupObjRef(ref *metav1.OwnerReference) (unidlingapi.CrossGroupObjectReference, error) {
gv, err := schema.ParseGroupVersion(ref.APIVersion)
if err != nil {
return unidlingapi.CrossGroupObjectReference{}, err
Expand All @@ -344,7 +312,7 @@ func makeCrossGroupObjRef(ref *kapi.ObjectReference) (unidlingapi.CrossGroupObje
// scalable objects by checking each address in each subset to see if it has a pod
// reference, and the following that pod reference to find the owning controller,
// and returning the unique set of controllers found this way.
func findScalableResourcesForEndpoints(endpoints *kapi.Endpoints, decoder runtime.Decoder, getPod func(kapi.ObjectReference) (*kapi.Pod, error), getController func(kapi.ObjectReference) (runtime.Object, error)) (map[unidlingapi.CrossGroupObjectReference]struct{}, error) {
func findScalableResourcesForEndpoints(endpoints *kapi.Endpoints, getPod func(kapi.ObjectReference) (*kapi.Pod, error), getController func(string, metav1.OwnerReference) (metav1.Object, error)) (map[unidlingapi.CrossGroupObjectReference]struct{}, error) {
// To find all RCs and DCs for an endpoint, we first figure out which pods are pointed to by that endpoint...
podRefs := map[kapi.ObjectReference]*kapi.Pod{}
for _, subset := range endpoints.Subsets {
Expand All @@ -363,33 +331,26 @@ func findScalableResourcesForEndpoints(endpoints *kapi.Endpoints, decoder runtim
}

// ... then, for each pod, we check the controller, and find the set of unique controllers...
immediateControllerRefs := make(map[kapi.ObjectReference]struct{})
immediateControllerRefs := make(map[metav1.OwnerReference]struct{})
for _, pod := range podRefs {
controllerRef, err := getControllerRef(pod, decoder)
if err != nil {
return nil, fmt.Errorf("unable to find controller for pod %s/%s: %v", pod.Namespace, pod.Name, err)
} else if controllerRef == nil {
controllerRef := metav1.GetControllerOf(pod)
if controllerRef == nil {
return nil, fmt.Errorf("unable to find controller for pod %s/%s: no creator reference listed", pod.Namespace, pod.Name)
}

immediateControllerRefs[*controllerRef] = struct{}{}
}

// ... finally, for each controller, we load it, and see if there is a corresponding owner (to cover cases like DCs, Deployments, etc)
controllerRefs := make(map[unidlingapi.CrossGroupObjectReference]struct{})
for controllerRef := range immediateControllerRefs {
controller, err := getController(controllerRef)
controller, err := getController(endpoints.Namespace, controllerRef)
if utilerrors.TolerateNotFoundError(err) != nil {
return nil, fmt.Errorf("unable to load %s %q: %v", controllerRef.Kind, controllerRef.Name, err)
}

if controller != nil {
var parentControllerRef *kapi.ObjectReference
parentControllerRef, err = getControllerRef(controller, decoder)
if err != nil {
return nil, fmt.Errorf("unable to load the creator of %s %q: %v", controllerRef.Kind, controllerRef.Name, err)
}

var parentControllerRef *metav1.OwnerReference
parentControllerRef = metav1.GetControllerOf(controller)
var crossGroupObjRef unidlingapi.CrossGroupObjectReference
if parentControllerRef == nil {
// if this is just a plain RC, use it
Expand Down Expand Up @@ -553,7 +514,7 @@ func (o *IdleOptions) RunIdle(f *clientcmd.Factory) error {
return err
}

externalKubeExtensionClient := kextensionsclient.New(kclient.Core().RESTClient())
externalKubeExtensionClient := kextensionsclient.New(kclient.Extensions().RESTClient())
delegScaleGetter := appsmanualclient.NewDelegatingScaleNamespacer(appsV1Client, externalKubeExtensionClient)
scaleAnnotater := utilunidling.NewScaleAnnotater(delegScaleGetter, appClient.Apps(), kclient.Core(), func(currentReplicas int32, annotations map[string]string) {
annotations[unidlingapi.IdledAtAnnotation] = nowTime.UTC().Format(time.RFC3339)
Expand Down
116 changes: 39 additions & 77 deletions pkg/oc/cli/cmd/idle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,54 +4,34 @@ import (
"encoding/json"
"testing"

appsapi "github.com/openshift/origin/pkg/apps/apis/apps"
unidlingapi "github.com/openshift/origin/pkg/unidling/api"

oappsapi "github.com/openshift/origin/pkg/apps/apis/apps"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
ktypes "k8s.io/apimachinery/pkg/types"
"k8s.io/kubernetes/pkg/api/legacyscheme"
kapi "k8s.io/kubernetes/pkg/apis/core"

oapi "github.com/openshift/origin/pkg/api"

// install all APIs
_ "github.com/openshift/origin/pkg/api/install"
_ "k8s.io/kubernetes/pkg/apis/core/install"
)

func makePod(name, rcName string, t *testing.T) kapi.Pod {
// this snippet is from kube's code to set the created-by annotation
// (which itself does not do quite what we want here)

codec := legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: kapi.GroupName, Version: "v1"})

createdByRefJson, err := kruntime.Encode(codec, &kapi.SerializedReference{
Reference: kapi.ObjectReference{
Kind: "ReplicationController",
Name: rcName,
Namespace: "somens",
},
})

if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

return kapi.Pod{
func makePod(name string, rc metav1.Object, t *testing.T) kapi.Pod {
pod := kapi.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "somens",
Annotations: map[string]string{
oapi.DeprecatedKubeCreatedByAnnotation: string(createdByRefJson),
},
},
}
pod.OwnerReferences = append(pod.OwnerReferences,
*metav1.NewControllerRef(rc, kapi.SchemeGroupVersion.WithKind("ReplicationController")))

return pod
}

func makeRC(name, dcName, createdByDCName string, t *testing.T) *kapi.ReplicationController {
func makeRC(name string, dc metav1.Object, t *testing.T) *kapi.ReplicationController {
rc := kapi.ReplicationController{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -60,25 +40,9 @@ func makeRC(name, dcName, createdByDCName string, t *testing.T) *kapi.Replicatio
},
}

if createdByDCName != "" {
codec := legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: kapi.GroupName, Version: "v1"})
createdByRefJson, err := kruntime.Encode(codec, &kapi.SerializedReference{
Reference: kapi.ObjectReference{
Kind: "DeploymentConfig",
Name: createdByDCName,
Namespace: "somens",
},
})

if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

rc.Annotations[oapi.DeprecatedKubeCreatedByAnnotation] = string(createdByRefJson)
}

if dcName != "" {
rc.Annotations[appsapi.DeploymentConfigAnnotation] = dcName
if dc != nil {
rc.OwnerReferences = append(rc.OwnerReferences, *metav1.NewControllerRef(dc,
oappsapi.SchemeGroupVersion.WithKind("DeploymentConfig")))
}

return &rc
Expand All @@ -92,12 +56,9 @@ func makePodRef(name string) *kapi.ObjectReference {
}
}

func makeRCRef(name string) *kapi.ObjectReference {
return &kapi.ObjectReference{
Kind: "ReplicationController",
Name: name,
Namespace: "somens",
}
func makeRCRef(name string) *metav1.OwnerReference {
return metav1.NewControllerRef(&metav1.ObjectMeta{Name: name},
kapi.SchemeGroupVersion.WithKind("ReplicationController"))
}

func TestFindIdlablesForEndpoints(t *testing.T) {
Expand Down Expand Up @@ -140,12 +101,19 @@ func TestFindIdlablesForEndpoints(t *testing.T) {
},
}

controllers := map[string]metav1.Object{
"somerc1": makeRC("somerc1", &metav1.ObjectMeta{Name: "somedc1"}, t),
"somerc2": makeRC("somerc2", nil, t),
"somerc3": makeRC("somerc3", &metav1.ObjectMeta{Name: "somedc2"}, t),
"somerc4": makeRC("somerc4", &metav1.ObjectMeta{Name: "somedc2"}, t),
}

pods := map[kapi.ObjectReference]kapi.Pod{
*makePodRef("somepod1"): makePod("somepod1", "somerc1", t),
*makePodRef("somepod2"): makePod("somepod2", "somerc2", t),
*makePodRef("somepod3"): makePod("somepod3", "somerc1", t),
*makePodRef("somepod4"): makePod("somepod4", "somerc3", t),
*makePodRef("somepod5"): makePod("somepod5", "somerc4", t),
*makePodRef("somepod1"): makePod("somepod1", controllers["somerc1"], t),
*makePodRef("somepod2"): makePod("somepod2", controllers["somerc2"], t),
*makePodRef("somepod3"): makePod("somepod3", controllers["somerc1"], t),
*makePodRef("somepod4"): makePod("somepod4", controllers["somerc3"], t),
*makePodRef("somepod5"): makePod("somepod5", controllers["somerc4"], t),
}

getPod := func(ref kapi.ObjectReference) (*kapi.Pod, error) {
Expand All @@ -155,16 +123,8 @@ func TestFindIdlablesForEndpoints(t *testing.T) {
return nil, kerrors.NewNotFound(schema.GroupResource{Group: kapi.GroupName, Resource: "Pod"}, ref.Name)
}

controllers := map[kapi.ObjectReference]kruntime.Object{
// prefer CreatedByAnnotation to DeploymentConfigAnnotation
*makeRCRef("somerc1"): makeRC("somerc1", "nonsense-value", "somedc1", t),
*makeRCRef("somerc2"): makeRC("somerc2", "", "", t),
*makeRCRef("somerc3"): makeRC("somerc3", "somedc2", "", t),
*makeRCRef("somerc4"): makeRC("somerc4", "", "somedc2", t),
}

getController := func(ref kapi.ObjectReference) (kruntime.Object, error) {
if controller, ok := controllers[ref]; ok {
getController := func(namespace string, ref metav1.OwnerReference) (metav1.Object, error) {
if controller, ok := controllers[ref.Name]; ok {
return controller, nil
}

Expand All @@ -174,25 +134,27 @@ func TestFindIdlablesForEndpoints(t *testing.T) {

}

codec := legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: kapi.GroupName, Version: "v1"})
refSet, err := findScalableResourcesForEndpoints(endpoints, codec, getPod, getController)
refSet, err := findScalableResourcesForEndpoints(endpoints, getPod, getController)

if err != nil {
t.Fatalf("Unexpected error while finding idlables: %v", err)
}

expectedRefs := []unidlingapi.CrossGroupObjectReference{
{
Kind: "DeploymentConfig",
Name: "somedc1",
Kind: "DeploymentConfig",
Name: "somedc1",
Group: oappsapi.GroupName,
},
{
Kind: "DeploymentConfig",
Name: "somedc2",
Kind: "DeploymentConfig",
Name: "somedc2",
Group: oappsapi.GroupName,
},
{
Kind: "ReplicationController",
Name: "somerc2",
Kind: "ReplicationController",
Name: "somerc2",
Group: kapi.GroupName,
},
}

Expand All @@ -202,7 +164,7 @@ func TestFindIdlablesForEndpoints(t *testing.T) {

for _, ref := range expectedRefs {
if _, ok := refSet[ref]; !ok {
t.Errorf("expected ReplicationController %q to be present, but was not", ref.Name)
t.Errorf("expected ReplicationController %q to be present, but was not in %v", ref.Name, refSet)
}
}
}
Expand Down

0 comments on commit 1533d3e

Please sign in to comment.