Skip to content

Commit

Permalink
Switch idling over to using owner references.
Browse files Browse the repository at this point in the history 
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 openshift#17798
  • Loading branch information
@DirectXMan12
DirectXMan12 committed Jan 17, 2018
1 parent 78ddc10 commit ab9cc38
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 78 deletions.
49 changes: 19 additions & 30 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 @@ -256,7 +254,6 @@ func (o *IdleOptions) calculateIdlableAnnotationsByService(f *clientcmd.Factory)
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 +268,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 @@ -294,37 +291,29 @@ func (o *IdleOptions) calculateIdlableAnnotationsByService(f *clientcmd.Factory)
}

// 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) {
// It will use the first OwnerReference that has Controller set to true. If none are found, it
// will return nil.
func getControllerRef(obj runtime.Object) (*kapi.ObjectReference, error) {
objMeta, err := meta.Accessor(obj)
if err != nil {
return nil, err
}

annotations := objMeta.GetAnnotations()
ownerRefs := objMeta.GetOwnerReferences()

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
for _, ownerRef := range ownerRefs {
// just consider the first such reference
if ownerRef.Controller != nil && *ownerRef.Controller == true {
return &kapi.ObjectReference{
Name: ownerRef.Name,
Namespace: objMeta.GetNamespace(),
Kind: ownerRef.Kind,
APIVersion: ownerRef.APIVersion,
}, 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
return nil, nil
}

func makeCrossGroupObjRef(ref *kapi.ObjectReference) (unidlingapi.CrossGroupObjectReference, error) {
Expand All @@ -344,7 +333,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(kapi.ObjectReference) (runtime.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 @@ -365,7 +354,7 @@ 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{})
for _, pod := range podRefs {
controllerRef, err := getControllerRef(pod, decoder)
controllerRef, err := getControllerRef(pod)
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 {
Expand All @@ -385,7 +374,7 @@ func findScalableResourcesForEndpoints(endpoints *kapi.Endpoints, decoder runtim

if controller != nil {
var parentControllerRef *kapi.ObjectReference
parentControllerRef, err = getControllerRef(controller, decoder)
parentControllerRef, err = getControllerRef(controller)
if err != nil {
return nil, fmt.Errorf("unable to load the creator of %s %q: %v", controllerRef.Kind, controllerRef.Name, err)
}
Expand Down
68 changes: 20 additions & 48 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,39 @@ import (
"encoding/json"
"testing"

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

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)
}

vTrue := true
return kapi.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "somens",
Annotations: map[string]string{
oapi.DeprecatedKubeCreatedByAnnotation: string(createdByRefJson),
OwnerReferences: []metav1.OwnerReference{
{
Kind: "ReplicationController",
Name: rcName,
Controller: &vTrue,
},
},
},
}
}

func makeRC(name, dcName, createdByDCName string, t *testing.T) *kapi.ReplicationController {
func makeRC(name, dcName string, t *testing.T) *kapi.ReplicationController {
vTrue := true
rc := kapi.ReplicationController{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -60,25 +45,14 @@ 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 dcName != "" {
rc.OwnerReferences = []metav1.OwnerReference{
{
Kind: "DeploymentConfig",
Name: dcName,
Controller: &vTrue,
},
})

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

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

if dcName != "" {
rc.Annotations[appsapi.DeploymentConfigAnnotation] = dcName
}

return &rc
Expand Down Expand Up @@ -156,11 +130,10 @@ func TestFindIdlablesForEndpoints(t *testing.T) {
}

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),
*makeRCRef("somerc1"): makeRC("somerc1", "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) {
Expand All @@ -174,8 +147,7 @@ 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)
Expand Down

0 comments on commit ab9cc38

Please sign in to comment.