From 247dd30d8987f77bdc3925d592c797a4753c5e20 Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 3 May 2018 14:37:06 -0400 Subject: [PATCH 1/2] remove old reapers --- .../cluster/app_create/setup_cleanup.go | 7 - pkg/oc/cli/builds/doc.go | 2 - pkg/oc/cli/builds/reaper.go | 150 --------- pkg/oc/cli/builds/reaper_test.go | 223 ------------- pkg/oc/cli/deploymentconfigs/delete.go | 144 --------- pkg/oc/cli/deploymentconfigs/delete_test.go | 295 ------------------ pkg/oc/cli/util/clientcmd/factory_builder.go | 26 -- 7 files changed, 847 deletions(-) delete mode 100644 pkg/oc/cli/builds/doc.go delete mode 100644 pkg/oc/cli/builds/reaper.go delete mode 100644 pkg/oc/cli/builds/reaper_test.go delete mode 100644 pkg/oc/cli/deploymentconfigs/delete.go delete mode 100644 pkg/oc/cli/deploymentconfigs/delete_test.go diff --git a/pkg/oc/admin/diagnostics/diagnostics/cluster/app_create/setup_cleanup.go b/pkg/oc/admin/diagnostics/diagnostics/cluster/app_create/setup_cleanup.go index 41376d163ba3..1d062d42bf5e 100644 --- a/pkg/oc/admin/diagnostics/diagnostics/cluster/app_create/setup_cleanup.go +++ b/pkg/oc/admin/diagnostics/diagnostics/cluster/app_create/setup_cleanup.go @@ -8,10 +8,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" - appsutil "github.com/openshift/origin/pkg/apps/util" "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" newproject "github.com/openshift/origin/pkg/oc/admin/project" - appscmd "github.com/openshift/origin/pkg/oc/cli/deploymentconfigs" ) const podGoneTimeout = 30 // seconds to wait for previous app pods to disappear @@ -82,11 +80,6 @@ func (d *AppCreate) cleanupApp() { errs := []error{} d.out.Debug("DCluAC043", fmt.Sprintf("%s: Deleting components of app '%s' if present.", now(), d.appName)) - // reap the DC's deployments first - if err := appscmd.NewDeploymentConfigReaper(d.AppsClient, d.KubeClient, appsutil.NewReplicationControllerV1ScaleClient(d.KubeClient)).Stop(d.project, d.appName, time.Duration(1)*time.Second, nil); err != nil { - errs = append(errs, err) - } - // then delete the DC, service, and route if err := d.AppsClient.Apps().DeploymentConfigs(d.project).Delete(d.appName, nil); err != nil { errs = append(errs, err) diff --git a/pkg/oc/cli/builds/doc.go b/pkg/oc/cli/builds/doc.go deleted file mode 100644 index 0535e6a419f5..000000000000 --- a/pkg/oc/cli/builds/doc.go +++ /dev/null @@ -1,2 +0,0 @@ -// Package cmd provides command helpers for builds -package builds diff --git a/pkg/oc/cli/builds/reaper.go b/pkg/oc/cli/builds/reaper.go deleted file mode 100644 index a998edc21639..000000000000 --- a/pkg/oc/cli/builds/reaper.go +++ /dev/null @@ -1,150 +0,0 @@ -package builds - -import ( - "sort" - "strings" - "time" - - "github.com/golang/glog" - kerrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ktypes "k8s.io/apimachinery/pkg/types" - kutilerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/client-go/util/retry" - "k8s.io/kubernetes/pkg/kubectl" - - buildapi "github.com/openshift/origin/pkg/build/apis/build" - buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset" - buildutil "github.com/openshift/origin/pkg/build/util" - "github.com/openshift/origin/pkg/util" -) - -// NewBuildConfigReaper returns a new reaper for buildConfigs -func NewBuildConfigReaper(buildClient buildclient.Interface) kubectl.Reaper { - return &BuildConfigReaper{buildClient: buildClient, pollInterval: kubectl.Interval, timeout: kubectl.Timeout} -} - -// BuildConfigReaper implements the Reaper interface for buildConfigs -type BuildConfigReaper struct { - buildClient buildclient.Interface - pollInterval, timeout time.Duration -} - -// Stop deletes the build configuration and all of the associated builds. -func (reaper *BuildConfigReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *metav1.DeleteOptions) error { - _, err := reaper.buildClient.Build().BuildConfigs(namespace).Get(name, metav1.GetOptions{}) - - if err != nil { - return err - } - - var bcPotentialBuilds []buildapi.Build - - // Collect builds related to the config. - builds, err := reaper.buildClient.Build().Builds(namespace).List(metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelector(name).String()}) - if err != nil { - return err - } - - bcPotentialBuilds = append(bcPotentialBuilds, builds.Items...) - - // Collect deprecated builds related to the config. - // TODO: Delete this block after BuildConfigLabelDeprecated is removed. - builds, err = reaper.buildClient.Build().Builds(namespace).List(metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelectorDeprecated(name).String()}) - if err != nil { - return err - } - - bcPotentialBuilds = append(bcPotentialBuilds, builds.Items...) - - // A map of builds associated with this build configuration - bcBuilds := make(map[ktypes.UID]buildapi.Build) - - // Because of name length limits in the BuildConfigSelector, annotations are used to ensure - // reliable selection of associated builds. - for _, build := range bcPotentialBuilds { - if build.Annotations != nil { - if bcName, ok := build.Annotations[buildapi.BuildConfigAnnotation]; ok { - // The annotation, if present, has the full build config name. - if bcName != name { - // If the name does not match exactly, the build is not truly associated with the build configuration - continue - } - } - } - // Note that if there is no annotation, this is a deprecated build spec - // and we choose to include it in the deletion having matched only the BuildConfigSelectorDeprecated - - // Use a map to union the lists returned by the contemporary & deprecated build queries - // (there will be overlap between the lists, and we only want to try to delete each build once) - bcBuilds[build.UID] = build - } - - // If there are builds associated with this build configuration, pause it before attempting the deletion - if len(bcBuilds) > 0 { - - // Add paused annotation to the build config pending the deletion - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - - bc, err := reaper.buildClient.Build().BuildConfigs(namespace).Get(name, metav1.GetOptions{}) - if err != nil { - return err - } - - // Ignore if the annotation already exists - if strings.ToLower(bc.Annotations[buildapi.BuildConfigPausedAnnotation]) == "true" { - return nil - } - - // Set the annotation and update - if err := util.AddObjectAnnotations(bc, map[string]string{buildapi.BuildConfigPausedAnnotation: "true"}); err != nil { - return err - } - _, err = reaper.buildClient.Build().BuildConfigs(namespace).Update(bc) - return err - }) - - if err != nil { - return err - } - - } - - // Warn the user if the BuildConfig won't get deleted after this point. - bcDeleted := false - defer func() { - if !bcDeleted { - glog.Warningf("BuildConfig %s/%s will not be deleted because not all associated builds could be deleted. You can try re-running the command or removing them manually", namespace, name) - } - }() - - // For the benefit of test cases, sort the UIDs so that the deletion order is deterministic - buildUIDs := make([]string, 0, len(bcBuilds)) - for buildUID := range bcBuilds { - buildUIDs = append(buildUIDs, string(buildUID)) - } - sort.Strings(buildUIDs) - - errList := []error{} - for _, buildUID := range buildUIDs { - build := bcBuilds[ktypes.UID(buildUID)] - if err := reaper.buildClient.Build().Builds(namespace).Delete(build.Name, &metav1.DeleteOptions{}); err != nil { - glog.Warningf("Cannot delete Build %s/%s: %v", build.Namespace, build.Name, err) - if !kerrors.IsNotFound(err) { - errList = append(errList, err) - } - } - } - - // Aggregate all errors - if len(errList) > 0 { - return kutilerrors.NewAggregate(errList) - } - - if err := reaper.buildClient.Build().BuildConfigs(namespace).Delete(name, &metav1.DeleteOptions{}); err != nil { - return err - } - - bcDeleted = true - return nil -} diff --git a/pkg/oc/cli/builds/reaper_test.go b/pkg/oc/cli/builds/reaper_test.go deleted file mode 100644 index 6e895f6d0d27..000000000000 --- a/pkg/oc/cli/builds/reaper_test.go +++ /dev/null @@ -1,223 +0,0 @@ -package builds - -import ( - "fmt" - "reflect" - "strings" - "testing" - "time" - - kerrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - ktypes "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" - clientgotesting "k8s.io/client-go/testing" - - buildapi "github.com/openshift/origin/pkg/build/apis/build" - buildfake "github.com/openshift/origin/pkg/build/generated/internalclientset/fake" - buildutil "github.com/openshift/origin/pkg/build/util" -) - -var ( - configName = strings.Repeat("a", validation.DNS1123LabelMaxLength) - longConfigNameA = strings.Repeat("0", 250) + "a" - longConfigNameB = strings.Repeat("0", 250) + "b" - buildsResource = schema.GroupVersionResource{Group: "build.openshift.io", Version: "", Resource: "builds"} - buildKind = schema.GroupVersionKind{Group: "build.openshift.io", Version: "", Kind: "Build"} - buildConfigsResource = schema.GroupVersionResource{Group: "build.openshift.io", Version: "", Resource: "buildconfigs"} - buildConfigKind = schema.GroupVersionKind{Group: "build.openshift.io", Version: "", Kind: "BuildConfig"} -) - -func makeBuildConfig(configName string, version int64, deleting bool) *buildapi.BuildConfig { - ret := &buildapi.BuildConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: configName, - Namespace: "default", - Annotations: make(map[string]string), - }, - Spec: buildapi.BuildConfigSpec{}, - Status: buildapi.BuildConfigStatus{ - LastVersion: version, - }, - } - if deleting { - ret.Annotations[buildapi.BuildConfigPausedAnnotation] = "true" - } - return ret -} - -func makeBuild(configName string, version int) buildapi.Build { - return buildapi.Build{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("build-%s-%d", configName, version), - UID: ktypes.UID(fmt.Sprintf("build-%s-%d", configName, version)), - Namespace: "default", - Labels: map[string]string{buildapi.BuildConfigLabel: buildapi.LabelValue(configName)}, - Annotations: map[string]string{buildapi.BuildConfigAnnotation: configName}, - }, - } -} - -func makeDeprecatedBuild(configName string, version int) buildapi.Build { - return buildapi.Build{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("build-%s-%d", configName, version), - UID: ktypes.UID(fmt.Sprintf("build-%s-%d", configName, version)), - Namespace: "default", - Labels: map[string]string{buildapi.BuildConfigLabelDeprecated: buildapi.LabelValue(configName)}, - Annotations: map[string]string{buildapi.BuildConfigAnnotation: configName}, - }, - } -} - -func makeBuildList(configName string, version int) *buildapi.BuildList { - if version%2 != 0 { - panic("version needs be even") - } - list := &buildapi.BuildList{} - - for i := 1; i <= version; i += 2 { - list.Items = append(list.Items, makeBuild(configName, i)) - list.Items = append(list.Items, makeDeprecatedBuild(configName, i+1)) - } - return list -} - -func newBuildListFake(objects ...runtime.Object) *buildfake.Clientset { - fake := buildfake.NewSimpleClientset(objects...) - fake.PrependReactor("list", "builds", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - selector := action.(clientgotesting.ListAction).GetListRestrictions().Labels - retList := &buildapi.BuildList{} - for _, obj := range objects { - list, ok := obj.(*buildapi.BuildList) - if !ok { - continue - } - for _, build := range list.Items { - if selector.Matches(labels.Set(build.Labels)) { - retList.Items = append(retList.Items, build) - } - } - } - return true, retList, nil - }) - return fake -} - -func actionsAreEqual(a, b clientgotesting.Action) bool { - if reflect.DeepEqual(a, b) { - return true - } - // If it's an update action, we will take a better look at the object - if a.GetVerb() == "update" && b.GetVerb() == "update" && - a.GetNamespace() == b.GetNamespace() && - a.GetResource() == b.GetResource() && - a.GetSubresource() == b.GetSubresource() { - ret := reflect.DeepEqual(a.(clientgotesting.UpdateAction).GetObject(), b.(clientgotesting.UpdateAction).GetObject()) - return ret - } - return false -} - -func TestStop(t *testing.T) { - notFoundClient := &buildfake.Clientset{} //(notFound(), makeBuildList(configName, 2)) - notFoundClient.AddReactor("*", "*", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, kerrors.NewNotFound(buildapi.Resource("BuildConfig"), configName) - }) - - tests := map[string]struct { - targetBC string - oc *buildfake.Clientset - expected []clientgotesting.Action - err bool - }{ - "simple stop": { - targetBC: configName, - oc: newBuildListFake(makeBuildConfig(configName, 0, false)), - expected: []clientgotesting.Action{ - clientgotesting.NewGetAction(buildConfigsResource, "default", configName), - // Since there are no builds associated with this build config, do not expect an update - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelector(configName).String()}), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelectorDeprecated(configName).String()}), - clientgotesting.NewDeleteAction(buildConfigsResource, "default", configName), - }, - err: false, - }, - "multiple builds": { - targetBC: configName, - oc: newBuildListFake(makeBuildConfig(configName, 4, false), makeBuildList(configName, 4)), - expected: []clientgotesting.Action{ - clientgotesting.NewGetAction(buildConfigsResource, "default", configName), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelector(configName).String()}), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelectorDeprecated(configName).String()}), - clientgotesting.NewGetAction(buildConfigsResource, "default", configName), // Second GET to enable conflict retry logic - clientgotesting.NewUpdateAction(buildConfigsResource, "default", makeBuildConfig(configName, 4, true)), // Because this bc has builds, it is paused - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+configName+"-1"), - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+configName+"-2"), - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+configName+"-3"), - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+configName+"-4"), - clientgotesting.NewDeleteAction(buildConfigsResource, "default", configName), - }, - err: false, - }, - "long name builds": { - targetBC: longConfigNameA, - oc: newBuildListFake(makeBuildConfig(longConfigNameA, 4, false), makeBuildList(longConfigNameA, 4), makeBuildList(longConfigNameB, 4)), - expected: []clientgotesting.Action{ - clientgotesting.NewGetAction(buildConfigsResource, "default", longConfigNameA), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelector(longConfigNameA).String()}), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelectorDeprecated(longConfigNameA).String()}), - clientgotesting.NewGetAction(buildConfigsResource, "default", longConfigNameA), // Second GET to enable conflict retry logic - clientgotesting.NewUpdateAction(buildConfigsResource, "default", makeBuildConfig(longConfigNameA, 4, true)), // Because this bc has builds, it is paused - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+longConfigNameA+"-1"), - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+longConfigNameA+"-2"), - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+longConfigNameA+"-3"), - clientgotesting.NewDeleteAction(buildsResource, "default", "build-"+longConfigNameA+"-4"), - clientgotesting.NewDeleteAction(buildConfigsResource, "default", longConfigNameA), - }, - err: false, - }, - "no config, no or some builds": { - targetBC: configName, - oc: notFoundClient, - expected: []clientgotesting.Action{ - clientgotesting.NewGetAction(buildConfigsResource, "default", configName), - }, - err: true, - }, - "config, no builds": { - targetBC: configName, - oc: buildfake.NewSimpleClientset(makeBuildConfig(configName, 0, false)), - expected: []clientgotesting.Action{ - clientgotesting.NewGetAction(buildConfigsResource, "default", configName), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelector(configName).String()}), - clientgotesting.NewListAction(buildsResource, buildKind, "default", metav1.ListOptions{LabelSelector: buildutil.BuildConfigSelectorDeprecated(configName).String()}), - clientgotesting.NewDeleteAction(buildConfigsResource, "default", configName), - }, - err: false, - }, - } - - for testName, test := range tests { - reaper := &BuildConfigReaper{buildClient: test.oc, pollInterval: time.Millisecond, timeout: time.Millisecond} - err := reaper.Stop("default", test.targetBC, 1*time.Second, nil) - - if !test.err && err != nil { - t.Errorf("%s: unexpected error: %v", testName, err) - } - if test.err && err == nil { - t.Errorf("%s: expected an error", testName) - } - if len(test.oc.Actions()) != len(test.expected) { - t.Fatalf("%s: unexpected actions: %v, expected %v", testName, test.oc.Actions(), test.expected) - } - for j, actualAction := range test.oc.Actions() { - if !actionsAreEqual(actualAction, test.expected[j]) { - t.Errorf("%s: unexpected action: %v, expected %v", testName, actualAction, test.expected[j]) - } - } - } -} diff --git a/pkg/oc/cli/deploymentconfigs/delete.go b/pkg/oc/cli/deploymentconfigs/delete.go deleted file mode 100644 index a6dc68a97ae8..000000000000 --- a/pkg/oc/cli/deploymentconfigs/delete.go +++ /dev/null @@ -1,144 +0,0 @@ -package deploymentconfigs - -import ( - "time" - - "github.com/golang/glog" - kerrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" - scaleclient "k8s.io/client-go/scale" - kapi "k8s.io/kubernetes/pkg/apis/core" - kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" - "k8s.io/kubernetes/pkg/kubectl" - - appsapi "github.com/openshift/origin/pkg/apps/apis/apps" - appsclient "github.com/openshift/origin/pkg/apps/generated/internalclientset" - "github.com/openshift/origin/pkg/apps/util" -) - -// NewDeploymentConfigReaper returns a new reaper for deploymentConfigs -func NewDeploymentConfigReaper(appsClient appsclient.Interface, kc kclientset.Interface, scaleClient scaleclient.ScalesGetter) kubectl.Reaper { - return &DeploymentConfigReaper{appsClient: appsClient, kc: kc, scaleClient: scaleClient, pollInterval: kubectl.Interval, timeout: kubectl.Timeout} -} - -// DeploymentConfigReaper implements the Reaper interface for deploymentConfigs -type DeploymentConfigReaper struct { - appsClient appsclient.Interface - kc kclientset.Interface - scaleClient scaleclient.ScalesGetter - pollInterval, timeout time.Duration -} - -// pause marks the deployment configuration as paused to avoid triggering new -// deployments. -func (reaper *DeploymentConfigReaper) pause(namespace, name string) (*appsapi.DeploymentConfig, error) { - patchBytes := []byte(`{"spec":{"paused":true,"replicas":0,"revisionHistoryLimit":0}}`) - return reaper.appsClient.Apps().DeploymentConfigs(namespace).Patch(name, types.StrategicMergePatchType, patchBytes) -} - -// Stop scales a replication controller via its deployment configuration down to -// zero replicas, waits for all of them to get deleted and then deletes both the -// replication controller and its deployment configuration. -func (reaper *DeploymentConfigReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *metav1.DeleteOptions) error { - // Pause the deployment configuration to prevent the new deployments from - // being triggered. - config, err := reaper.pause(namespace, name) - configNotFound := kerrors.IsNotFound(err) - if err != nil && !configNotFound { - return err - } - - var ( - isPaused bool - legacy bool - ) - // Determine if the deployment config controller noticed the pause. - if !configNotFound { - if err := wait.Poll(1*time.Second, 1*time.Minute, func() (bool, error) { - dc, err := reaper.appsClient.Apps().DeploymentConfigs(namespace).Get(name, metav1.GetOptions{}) - if err != nil { - return false, err - } - isPaused = dc.Spec.Paused - return dc.Status.ObservedGeneration >= config.Generation, nil - }); err != nil { - return err - } - - // If we failed to pause the deployment config, it means we are talking to - // old API that does not support pausing. In that case, we delete the - // deployment config to stay backward compatible. - if !isPaused { - if err := reaper.appsClient.Apps().DeploymentConfigs(namespace).Delete(name, &metav1.DeleteOptions{}); err != nil { - return err - } - // Setting this to true avoid deleting the config at the end. - legacy = true - } - } - - // Clean up deployments related to the config. Even if the deployment - // configuration has been deleted, we want to sweep the existing replication - // controllers and clean them up. - options := metav1.ListOptions{LabelSelector: util.ConfigSelector(name).String()} - rcList, err := reaper.kc.Core().ReplicationControllers(namespace).List(options) - if err != nil { - return err - } - - // If there is neither a config nor any deployments, nor any deployer pods, we can return NotFound. - deployments := rcList.Items - - if configNotFound && len(deployments) == 0 { - return kerrors.NewNotFound(kapi.Resource("deploymentconfig"), name) - } - - for _, rc := range deployments { - // this is unnecessary since the ownership is present - if reaper.scaleClient != nil { - rcReaper, err := kubectl.ReaperFor(kapi.Kind("ReplicationController"), reaper.kc, reaper.scaleClient) - if err != nil { - return err - } - if err = rcReaper.Stop(rc.Namespace, rc.Name, timeout, gracePeriod); err != nil { - // Better not error out here... - glog.Infof("Cannot delete ReplicationController %s/%s for deployment config %s/%s: %v", rc.Namespace, rc.Name, namespace, name, err) - } - } else { - if err := reaper.kc.Core().ReplicationControllers(rc.Namespace).Delete(rc.Name, nil); err != nil { - glog.Infof("Cannot delete ReplicationController %s/%s for deployment config %s/%s: %v", rc.Namespace, rc.Name, namespace, name, err) - } - } - - // Only remove deployer pods when the deployment was failed. For completed - // deployment the pods should be already deleted. - if !util.IsFailedDeployment(&rc) { - continue - } - - // Delete all deployer and hook pods - podList, err := reaper.kc.Core().Pods(rc.Namespace).List(metav1.ListOptions{ - LabelSelector: util.DeployerPodSelector(rc.Name).String(), - }) - if err != nil { - return err - } - for _, pod := range podList.Items { - err := reaper.kc.Core().Pods(pod.Namespace).Delete(pod.Name, gracePeriod) - if err != nil { - // Better not error out here... - glog.Infof("Cannot delete lifecycle Pod %s/%s for deployment config %s/%s: %v", pod.Namespace, pod.Name, namespace, name, err) - } - } - } - - // Nothing to delete or we already deleted the deployment config because we - // failed to pause. - if configNotFound || legacy { - return nil - } - - return reaper.appsClient.Apps().DeploymentConfigs(namespace).Delete(name, &metav1.DeleteOptions{}) -} diff --git a/pkg/oc/cli/deploymentconfigs/delete_test.go b/pkg/oc/cli/deploymentconfigs/delete_test.go deleted file mode 100644 index 70bbe51748b2..000000000000 --- a/pkg/oc/cli/deploymentconfigs/delete_test.go +++ /dev/null @@ -1,295 +0,0 @@ -package deploymentconfigs - -import ( - "reflect" - "testing" - "time" - - kapierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/diff" - clientgotesting "k8s.io/client-go/testing" - "k8s.io/kubernetes/pkg/api/legacyscheme" - kapi "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" - - appsapi "github.com/openshift/origin/pkg/apps/apis/apps" - _ "github.com/openshift/origin/pkg/apps/apis/apps/install" - appstest "github.com/openshift/origin/pkg/apps/apis/apps/test" - appsfake "github.com/openshift/origin/pkg/apps/generated/internalclientset/fake" - appsutil "github.com/openshift/origin/pkg/apps/util" -) - -func mkdeployment(version int64) kapi.ReplicationController { - deployment, _ := appsutil.MakeDeployment(appstest.OkDeploymentConfig(version), legacyscheme.Codecs.LegacyCodec(appsapi.SchemeGroupVersion)) - return *deployment -} - -func mkdeploymentlist(versions ...int64) *kapi.ReplicationControllerList { - list := &kapi.ReplicationControllerList{} - for _, v := range versions { - list.Items = append(list.Items, mkdeployment(v)) - } - return list -} - -func TestStop(t *testing.T) { - var ( - deploymentConfigsResource = schema.GroupVersionResource{Group: "apps.openshift.io", Resource: "deploymentconfigs"} - replicationControllersResource = schema.GroupVersionResource{Resource: "replicationcontrollers"} - replicationControllerKind = schema.GroupVersionKind{Kind: "ReplicationController"} - ) - - pauseBytes := []byte(`{"spec":{"paused":true,"replicas":0,"revisionHistoryLimit":0}}`) - - fakeDC := map[string]*appsapi.DeploymentConfig{ - "simple-stop": appstest.OkDeploymentConfig(1), - "legacy-simple-stop": appstest.OkDeploymentConfig(1), - "multi-stop": appstest.OkDeploymentConfig(5), - "legacy-multi-stop": appstest.OkDeploymentConfig(5), - "no-deployments": appstest.OkDeploymentConfig(5), - "legacy-no-deployments": appstest.OkDeploymentConfig(5), - } - - emptyClientset := func() *appsfake.Clientset { - result := &appsfake.Clientset{} - result.AddReactor("patch", "deploymentconfigs", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, kapierrors.NewNotFound(schema.GroupResource{Group: "apps.openshift.io", Resource: "deploymentconfig"}, "config") - }) - return result - } - - tests := []struct { - testName string - namespace string - name string - oc *appsfake.Clientset - kc *fake.Clientset - expected []clientgotesting.Action - kexpected []clientgotesting.Action - err bool - }{ - { - testName: "simple stop", - namespace: "default", - name: "config", - oc: appsfake.NewSimpleClientset(fakeDC["simple-stop"]), - kc: fake.NewSimpleClientset(mkdeploymentlist(1)), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"), - clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-1"), - }, - err: false, - }, - { - testName: "legacy simple stop", - namespace: "default", - name: "config", - oc: appsfake.NewSimpleClientset(fakeDC["legacy-simple-stop"]), - kc: fake.NewSimpleClientset(mkdeploymentlist(1)), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"), - clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-1"), - }, - err: false, - }, - { - testName: "stop multiple controllers", - namespace: "default", - name: "config", - oc: appsfake.NewSimpleClientset(fakeDC["multi-stop"]), - kc: fake.NewSimpleClientset(mkdeploymentlist(1, 2, 3, 4, 5)), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"), - clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-1"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-2"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-3"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-4"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-5"), - }, - err: false, - }, - { - testName: "legacy stop multiple controllers", - namespace: "default", - name: "config", - oc: appsfake.NewSimpleClientset(fakeDC["legacy-multi-stop"]), - kc: fake.NewSimpleClientset(mkdeploymentlist(1, 2, 3, 4, 5)), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"), - clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-1"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-2"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-3"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-4"), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-5"), - }, - err: false, - }, - { - testName: "no config, some deployments", - namespace: "default", - name: "config", - oc: emptyClientset(), - kc: fake.NewSimpleClientset(mkdeploymentlist(1)), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - clientgotesting.NewDeleteAction(replicationControllersResource, "default", "config-1"), - }, - err: false, - }, - { - testName: "no config, no deployments", - namespace: "default", - name: "config", - oc: emptyClientset(), - kc: fake.NewSimpleClientset(&kapi.ReplicationControllerList{}), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - }, - err: true, - }, - { - testName: "config, no deployments", - namespace: "default", - name: "config", - oc: appsfake.NewSimpleClientset(fakeDC["no-deployments"]), - kc: fake.NewSimpleClientset(&kapi.ReplicationControllerList{}), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"), - clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - }, - err: false, - }, - { - testName: "legacy config, no deployments", - namespace: "default", - name: "config", - oc: appsfake.NewSimpleClientset(fakeDC["legacy-no-deployments"]), - kc: fake.NewSimpleClientset(&kapi.ReplicationControllerList{}), - expected: []clientgotesting.Action{ - clientgotesting.NewPatchAction(deploymentConfigsResource, "default", "config", pauseBytes), - clientgotesting.NewGetAction(deploymentConfigsResource, "default", "config"), - clientgotesting.NewDeleteAction(deploymentConfigsResource, "default", "config"), - }, - kexpected: []clientgotesting.Action{ - clientgotesting.NewListAction(replicationControllersResource, replicationControllerKind, "default", metav1.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{"openshift.io/deployment-config.name": "config"}).String()}), - }, - err: false, - }, - } - - for _, test := range tests { - reaper := &DeploymentConfigReaper{appsClient: test.oc, kc: test.kc, pollInterval: time.Millisecond, timeout: time.Millisecond} - err := reaper.Stop(test.namespace, test.name, 1*time.Second, nil) - - if !test.err && err != nil { - t.Errorf("%s: unexpected error: %v", test.testName, err) - } - if test.err && err == nil { - t.Errorf("%s: expected an error", test.testName) - } - if len(test.oc.Actions()) != len(test.expected) { - t.Errorf("%s: unexpected actions: %s", test.testName, diff.ObjectReflectDiff(test.oc.Actions(), test.expected)) - continue - } - for j, actualAction := range test.oc.Actions() { - if j >= len(test.expected) { - break - } - e, a := test.expected[j], actualAction - switch a.(type) { - case clientgotesting.UpdateAction: - if e.GetVerb() != a.GetVerb() || - e.GetNamespace() != a.GetNamespace() || - e.GetResource() != a.GetResource() || - e.GetSubresource() != a.GetSubresource() { - t.Errorf("%s: unexpected action[%d]: expected %s, got %s", test.testName, j, e, a) - } - default: - if !reflect.DeepEqual(a, e) { - t.Errorf("%s: unexpected action[%d]: %s", test.testName, j, diff.ObjectReflectDiff(a, e)) - } - } - } - - if len(test.oc.Actions()) < len(test.expected) { - t.Errorf("%s: missing expected actions:", test.testName) - for _, action := range test.expected[len(test.oc.Actions()):] { - t.Errorf("\t%s\n", action) - } - continue - } else if len(test.oc.Actions()) > len(test.expected) { - t.Errorf("%s: unexpected additional actions:", test.testName) - for _, action := range test.oc.Actions()[len(test.expected):] { - t.Errorf("\t%s\n", action) - } - continue - } - - for j, actualAction := range test.kc.Actions() { - if j >= len(test.kexpected) { - break - } - e, a := test.kexpected[j], actualAction - switch a.(type) { - case clientgotesting.UpdateAction: - if e.GetVerb() != a.GetVerb() || - e.GetNamespace() != a.GetNamespace() || - e.GetResource() != a.GetResource() || - e.GetSubresource() != a.GetSubresource() { - t.Errorf("%s: unexpected action[%d]: expected %s, got %s", test.testName, j, e, a) - } - default: - if !reflect.DeepEqual(a, e) { - t.Errorf("%s: unexpected action[%d]: %s", test.testName, j, diff.ObjectReflectDiff(a, e)) - } - } - } - if len(test.kc.Actions()) < len(test.kexpected) { - t.Errorf("%s: missing expected actions:", test.testName) - for _, action := range test.kexpected[len(test.kc.Actions()):] { - t.Errorf("\t%s\n", action) - } - continue - } else if len(test.kc.Actions()) > len(test.kexpected) { - t.Errorf("%s: unexpected additional actions:", test.testName) - for _, action := range test.kc.Actions()[len(test.kexpected):] { - t.Errorf("\t%s\n", action) - } - continue - } - } -} diff --git a/pkg/oc/cli/util/clientcmd/factory_builder.go b/pkg/oc/cli/util/clientcmd/factory_builder.go index 013b1b884c56..2acb513ca4c9 100644 --- a/pkg/oc/cli/util/clientcmd/factory_builder.go +++ b/pkg/oc/cli/util/clientcmd/factory_builder.go @@ -8,16 +8,10 @@ import ( "k8s.io/kubernetes/pkg/kubectl/plugins" "k8s.io/kubernetes/pkg/kubectl/resource" - appsapi "github.com/openshift/origin/pkg/apps/apis/apps" - appsclient "github.com/openshift/origin/pkg/apps/generated/internalclientset" authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" authorizationclientinternal "github.com/openshift/origin/pkg/authorization/generated/internalclientset" authorizationreaper "github.com/openshift/origin/pkg/authorization/reaper" - buildapi "github.com/openshift/origin/pkg/build/apis/build" - buildclientinternal "github.com/openshift/origin/pkg/build/generated/internalclientset" oauthclientinternal "github.com/openshift/origin/pkg/oauth/generated/internalclientset" - buildcmd "github.com/openshift/origin/pkg/oc/cli/builds" - deploymentcmd "github.com/openshift/origin/pkg/oc/cli/deploymentconfigs" securityclientinternal "github.com/openshift/origin/pkg/security/generated/internalclientset" userapi "github.com/openshift/origin/pkg/user/apis/user" userclientinternal "github.com/openshift/origin/pkg/user/generated/internalclientset" @@ -72,20 +66,6 @@ func (f *ring2Factory) Reaper(mapping *meta.RESTMapping) (kubectl.Reaper, error) gk := mapping.GroupVersionKind.GroupKind() switch { - case appsapi.IsKindOrLegacy("DeploymentConfig", gk): - kc, err := f.clientAccessFactory.ClientSet() - if err != nil { - return nil, err - } - config, err := f.clientAccessFactory.ClientConfig() - if err != nil { - return nil, err - } - scaleClient, err := f.ScaleClient() - if err != nil { - return nil, err - } - return deploymentcmd.NewDeploymentConfigReaper(appsclient.NewForConfigOrDie(config), kc, scaleClient), nil case authorizationapi.IsKindOrLegacy("Role", gk): authClient, err := authorizationclientinternal.NewForConfig(clientConfig) if err != nil { @@ -142,12 +122,6 @@ func (f *ring2Factory) Reaper(mapping *meta.RESTMapping) (kubectl.Reaper, error) authClient, securityClient.Security().SecurityContextConstraints(), ), nil - case buildapi.IsKindOrLegacy("BuildConfig", gk): - config, err := f.clientAccessFactory.ClientConfig() - if err != nil { - return nil, err - } - return buildcmd.NewBuildConfigReaper(buildclientinternal.NewForConfigOrDie(config)), nil } return f.kubeBuilderFactory.Reaper(mapping) } From 3ddc5c7dc9059fe0242e5989e32634a54974e42c Mon Sep 17 00:00:00 2001 From: David Eads Date: Mon, 7 May 2018 10:51:09 -0400 Subject: [PATCH 2/2] UPSTREAM: 63490: default the ignorenotfound for delete when selecting objects --- vendor/k8s.io/kubernetes/pkg/kubectl/cmd/delete.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/delete.go b/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/delete.go index 86f8869abc67..784a9bd3935d 100644 --- a/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/delete.go +++ b/vendor/k8s.io/kubernetes/pkg/kubectl/cmd/delete.go @@ -191,13 +191,13 @@ func (o *DeleteOptions) Validate(cmd *cobra.Command) error { if o.DeleteAll && len(o.Selector) > 0 { return fmt.Errorf("cannot set --all and --selector at the same time") } - if o.DeleteAll { + if o.DeleteAll || len(o.Selector) > 0 { f := cmd.Flags().Lookup("ignore-not-found") // The flag should never be missing if f == nil { return fmt.Errorf("missing --ignore-not-found flag") } - // If the user didn't explicitly set the option, default to ignoring NotFound errors when used with --all + // If the user didn't explicitly set the option, default to ignoring NotFound errors when used with --all or -l if !f.Changed { o.IgnoreNotFound = true }