Merge pull request #19070 from deads2k/server-08-features-on

Automatic merge from submit-queue.

 remove disabledfeatures from master config

1. `FeatureBuilder` was agreed to remove by @smarterclayton and @eparis .
2. `FeatureS2I` was not used
3. `FeatureWebConsole` doesn't make sense the webconsole isn't include here.  Just don't install it.  Also, it didn't control the exposure of the console
4. `openshift.io/resource` used by image builder.  Don't know when this was added, but it never passed validation here: `ValidateDisabledFeatures`

@openshift/api-review 
@openshift/sig-master 
/assign @mfojtik 
/assign @soltysh

hack/test-cmd.sh

@@ -77,10 +77,6 @@ os::cmd::expect_success_and_not_text "KUBECONFIG='${MASTER_CONFIG_DIR}/admin.kub
 os::cmd::expect_success_and_not_text "KUBECONFIG='' oc version" "Missing or incomplete configuration info"
 os::test::junit::declare_suite_end
 
-os::test::junit::declare_suite_start "cmd/config"
-os::cmd::expect_success_and_text "cat ${MASTER_CONFIG_DIR}/master-config.yaml" 'disabledFeatures: null'
-os::test::junit::declare_suite_end
-
 os::start::master
 
 # profile the cli commands

pkg/cmd/openshift-apiserver/server.go

@@ -1,8 +1,6 @@
 package openshift_apiserver
 
 import (
-	"strings"
-
 	"github.com/golang/glog"
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -55,9 +53,6 @@ func RunOpenShiftAPIServer(masterConfig *configapi.MasterConfig) error {
 
 	glog.Infof("Starting master on %s (%s)", masterConfig.ServingInfo.BindAddress, version.Get().String())
 	glog.Infof("Public master address is %s", masterConfig.MasterPublicURL)
-	if len(masterConfig.DisabledFeatures) > 0 {
-		glog.V(4).Infof("Disabled features: %s", strings.Join(masterConfig.DisabledFeatures, ", "))
-	}
 	imageTemplate := variable.NewDefaultImageTemplate()
 	imageTemplate.Format = masterConfig.ImageConfig.Format
 	imageTemplate.Latest = masterConfig.ImageConfig.Latest

pkg/cmd/openshift-controller-manager/controller/config.go

@@ -58,7 +58,6 @@ type OpenshiftControllerConfig struct {
 	DeployerControllerConfig         DeployerControllerConfig
 	DeploymentConfigControllerConfig DeploymentConfigControllerConfig
 
-	ImageTriggerControllerConfig         ImageTriggerControllerConfig
 	ImageSignatureImportControllerConfig ImageSignatureImportControllerConfig
 	ImageImportControllerConfig          ImageImportControllerConfig
 
@@ -90,7 +89,7 @@ func (c *OpenshiftControllerConfig) GetControllerInitializers() (map[string]Init
 	ret["openshift.io/deployer"] = c.DeployerControllerConfig.RunController
 	ret["openshift.io/deploymentconfig"] = c.DeploymentConfigControllerConfig.RunController
 
-	ret["openshift.io/image-trigger"] = c.ImageTriggerControllerConfig.RunController
+	ret["openshift.io/image-trigger"] = RunImageTriggerController
 	ret["openshift.io/image-import"] = c.ImageImportControllerConfig.RunController
 	ret["openshift.io/image-signature-import"] = c.ImageSignatureImportControllerConfig.RunController
 
@@ -146,14 +145,6 @@ func BuildOpenshiftControllerConfig(options configapi.MasterConfig) (*OpenshiftC
 		Codec: annotationCodec,
 	}
 
-	ret.ImageTriggerControllerConfig = ImageTriggerControllerConfig{
-		HasBuilderEnabled: options.DisabledFeatures.Has(configapi.FeatureBuilder),
-		// TODO: make these consts in configapi
-		HasDeploymentsEnabled:  options.DisabledFeatures.Has("triggers.image.openshift.io/deployments"),
-		HasDaemonSetsEnabled:   options.DisabledFeatures.Has("triggers.image.openshift.io/daemonsets"),
-		HasStatefulSetsEnabled: options.DisabledFeatures.Has("triggers.image.openshift.io/statefulsets"),
-		HasCronJobsEnabled:     options.DisabledFeatures.Has("triggers.image.openshift.io/cronjobs"),
-	}
 	ret.ImageImportControllerConfig = ImageImportControllerConfig{
 		MaxScheduledImageImportsPerMinute:          options.ImagePolicyConfig.MaxScheduledImageImportsPerMinute,
 		ResyncPeriod:                               10 * time.Minute,

pkg/cmd/openshift-controller-manager/controller/image.go

@@ -27,16 +27,7 @@ import (
 	triggerdeploymentconfigs "github.com/openshift/origin/pkg/image/trigger/deploymentconfigs"
 )
 
-type ImageTriggerControllerConfig struct {
-	HasBuilderEnabled      bool
-	HasDeploymentsEnabled  bool
-	HasDaemonSetsEnabled   bool
-	HasStatefulSetsEnabled bool
-	HasCronJobsEnabled     bool
-}
-
-func (c *ImageTriggerControllerConfig) RunController(ctx ControllerContext) (bool, error) {
-	//streamInformer := ctx.ImageInformers.Image().InternalVersion().ImageStreams().Informer()
+func RunImageTriggerController(ctx ControllerContext) (bool, error) {
 	informer := ctx.ImageInformers.Image().InternalVersion().ImageStreams()
 
 	buildClient, err := ctx.ClientBuilder.OpenshiftInternalBuildClient(bootstrappolicy.InfraImageTriggerControllerServiceAccountName)
@@ -63,51 +54,41 @@ func (c *ImageTriggerControllerConfig) RunController(ctx ControllerContext) (boo
 			Reactor:   &triggerdeploymentconfigs.DeploymentConfigReactor{Client: appsClient.Apps()},
 		},
 	}
-	if !c.HasBuilderEnabled {
-		sources = append(sources, imagetriggercontroller.TriggerSource{
-			Resource:  schema.GroupResource{Group: "build.openshift.io", Resource: "buildconfigs"},
-			Informer:  ctx.BuildInformers.Build().InternalVersion().BuildConfigs().Informer(),
-			Store:     ctx.BuildInformers.Build().InternalVersion().BuildConfigs().Informer().GetIndexer(),
-			TriggerFn: triggerbuildconfigs.NewBuildConfigTriggerIndexer,
-			Reactor:   triggerbuildconfigs.NewBuildConfigReactor(bcInstantiator, kclient.Core().RESTClient()),
-		})
-	}
-	if !c.HasDeploymentsEnabled {
-		sources = append(sources, imagetriggercontroller.TriggerSource{
-			Resource:  schema.GroupResource{Group: "extensions", Resource: "deployments"},
-			Informer:  ctx.ExternalKubeInformers.Extensions().V1beta1().Deployments().Informer(),
-			Store:     ctx.ExternalKubeInformers.Extensions().V1beta1().Deployments().Informer().GetIndexer(),
-			TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
-			Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
-		})
-	}
-	if !c.HasDaemonSetsEnabled {
-		sources = append(sources, imagetriggercontroller.TriggerSource{
-			Resource:  schema.GroupResource{Group: "extensions", Resource: "daemonsets"},
-			Informer:  ctx.ExternalKubeInformers.Extensions().V1beta1().DaemonSets().Informer(),
-			Store:     ctx.ExternalKubeInformers.Extensions().V1beta1().DaemonSets().Informer().GetIndexer(),
-			TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
-			Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
-		})
-	}
-	if !c.HasStatefulSetsEnabled {
-		sources = append(sources, imagetriggercontroller.TriggerSource{
-			Resource:  schema.GroupResource{Group: "apps", Resource: "statefulsets"},
-			Informer:  ctx.ExternalKubeInformers.Apps().V1beta1().StatefulSets().Informer(),
-			Store:     ctx.ExternalKubeInformers.Apps().V1beta1().StatefulSets().Informer().GetIndexer(),
-			TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
-			Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
-		})
-	}
-	if !c.HasCronJobsEnabled {
-		sources = append(sources, imagetriggercontroller.TriggerSource{
-			Resource:  schema.GroupResource{Group: "batch", Resource: "cronjobs"},
-			Informer:  ctx.ExternalKubeInformers.Batch().V2alpha1().CronJobs().Informer(),
-			Store:     ctx.ExternalKubeInformers.Batch().V2alpha1().CronJobs().Informer().GetIndexer(),
-			TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
-			Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
-		})
-	}
+	sources = append(sources, imagetriggercontroller.TriggerSource{
+		Resource:  schema.GroupResource{Group: "build.openshift.io", Resource: "buildconfigs"},
+		Informer:  ctx.BuildInformers.Build().InternalVersion().BuildConfigs().Informer(),
+		Store:     ctx.BuildInformers.Build().InternalVersion().BuildConfigs().Informer().GetIndexer(),
+		TriggerFn: triggerbuildconfigs.NewBuildConfigTriggerIndexer,
+		Reactor:   triggerbuildconfigs.NewBuildConfigReactor(bcInstantiator, kclient.Core().RESTClient()),
+	})
+	sources = append(sources, imagetriggercontroller.TriggerSource{
+		Resource:  schema.GroupResource{Group: "extensions", Resource: "deployments"},
+		Informer:  ctx.ExternalKubeInformers.Extensions().V1beta1().Deployments().Informer(),
+		Store:     ctx.ExternalKubeInformers.Extensions().V1beta1().Deployments().Informer().GetIndexer(),
+		TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
+		Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
+	})
+	sources = append(sources, imagetriggercontroller.TriggerSource{
+		Resource:  schema.GroupResource{Group: "extensions", Resource: "daemonsets"},
+		Informer:  ctx.ExternalKubeInformers.Extensions().V1beta1().DaemonSets().Informer(),
+		Store:     ctx.ExternalKubeInformers.Extensions().V1beta1().DaemonSets().Informer().GetIndexer(),
+		TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
+		Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
+	})
+	sources = append(sources, imagetriggercontroller.TriggerSource{
+		Resource:  schema.GroupResource{Group: "apps", Resource: "statefulsets"},
+		Informer:  ctx.ExternalKubeInformers.Apps().V1beta1().StatefulSets().Informer(),
+		Store:     ctx.ExternalKubeInformers.Apps().V1beta1().StatefulSets().Informer().GetIndexer(),
+		TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
+		Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
+	})
+	sources = append(sources, imagetriggercontroller.TriggerSource{
+		Resource:  schema.GroupResource{Group: "batch", Resource: "cronjobs"},
+		Informer:  ctx.ExternalKubeInformers.Batch().V2alpha1().CronJobs().Informer(),
+		Store:     ctx.ExternalKubeInformers.Batch().V2alpha1().CronJobs().Informer().GetIndexer(),
+		TriggerFn: triggerannotations.NewAnnotationTriggerIndexer,
+		Reactor:   &triggerannotations.AnnotationReactor{Updater: updater},
+	})
 
 	go imagetriggercontroller.NewTriggerController(
 		broadcaster,

pkg/cmd/openshift-controller-manager/controller_manager.go

@@ -4,15 +4,13 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/golang/glog"
 
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kutilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientgoclientset "k8s.io/client-go/kubernetes"
 	kclientsetexternal "k8s.io/client-go/kubernetes"
@@ -63,9 +61,6 @@ func runOpenShiftControllerManager(masterConfig *configapi.MasterConfig, runServ
 	// you can't double run healthz, so only do this next bit if we aren't starting the API
 	if runServer {
 		glog.Infof("Starting controllers on %s (%s)", masterConfig.ServingInfo.BindAddress, version.Get().String())
-		if len(masterConfig.DisabledFeatures) > 0 {
-			glog.V(4).Infof("Disabled features: %s", strings.Join(masterConfig.DisabledFeatures, ", "))
-		}
 
 		if err := origincontrollers.RunControllerServer(masterConfig.ServingInfo, clientGoKubeExternal); err != nil {
 			return err
@@ -280,14 +275,7 @@ func startControllers(options configapi.MasterConfig, allocationController origi
 		return err
 	}
 
-	excludedControllers := getExcludedControllers(options)
-
 	for controllerName, initFn := range openshiftControllerInitializers {
-		// TODO remove this.  Only call one to start to prove the principle
-		if excludedControllers.Has(controllerName) {
-			glog.Warningf("%q is skipped", controllerName)
-			continue
-		}
 		if !controllerContext.IsControllerEnabled(controllerName) {
 			glog.Warningf("%q is disabled", controllerName)
 			continue
@@ -310,12 +298,3 @@ func startControllers(options configapi.MasterConfig, allocationController origi
 
 	return nil
 }
-
-func getExcludedControllers(options configapi.MasterConfig) sets.String {
-	excludedControllers := sets.NewString()
-	if !configapi.IsBuildEnabled(&options) {
-		excludedControllers.Insert("openshift.io/build")
-		excludedControllers.Insert("openshift.io/build-config-change")
-	}
-	return excludedControllers
-}

pkg/cmd/openshift-kube-apiserver/server.go

@@ -2,7 +2,6 @@ package openshift_kube_apiserver
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/golang/glog"
 
@@ -67,9 +66,6 @@ func RunOpenShiftKubeAPIServerServer(masterConfig *configapi.MasterConfig) error
 
 	glog.Infof("Starting master on %s (%s)", masterConfig.ServingInfo.BindAddress, version.Get().String())
 	glog.Infof("Public master address is %s", masterConfig.MasterPublicURL)
-	if len(masterConfig.DisabledFeatures) > 0 {
-		glog.V(4).Infof("Disabled features: %s", strings.Join(masterConfig.DisabledFeatures, ", "))
-	}
 	imageTemplate := variable.NewDefaultImageTemplate()
 	imageTemplate.Format = masterConfig.ImageConfig.Format
 	imageTemplate.Latest = masterConfig.ImageConfig.Latest

pkg/cmd/server/apis/config/helpers.go

@@ -23,80 +23,6 @@ import (
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 )
 
-var (
-	// Maps lower-cased feature flag names and aliases to their canonical names.
-	knownOpenShiftFeatureSet map[string]string
-)
-
-func init() {
-	knownOpenShiftFeatureSet = make(map[string]string, len(KnownOpenShiftFeatures))
-	for _, feature := range KnownOpenShiftFeatures {
-		knownOpenShiftFeatureSet[strings.ToLower(feature)] = feature
-	}
-	for alias, feature := range FeatureAliases {
-		knownOpenShiftFeatureSet[strings.ToLower(alias)] = feature
-	}
-}
-
-// NormalizeOpenShiftFeature returns canonical name for given OpenShift feature
-// flag or an alias if known. Otherwise lower-cased name is returned.
-func NormalizeOpenShiftFeature(name string) (string, bool) {
-	name = strings.ToLower(name)
-	if feature, ok := knownOpenShiftFeatureSet[name]; ok {
-		return feature, true
-	}
-	return name, false
-}
-
-// Add extends feature list with given valid items. They are appended
-// unless already present.
-func (fl *FeatureList) Add(items ...string) error {
-	unknown := []string{}
-	toAppend := make([]string, 0, len(items))
-	for _, item := range items {
-		feature, known := NormalizeOpenShiftFeature(item)
-		if !known {
-			unknown = append(unknown, item)
-			continue
-		}
-		if fl.Has(feature) {
-			continue
-		}
-		toAppend = append(toAppend, feature)
-	}
-	if len(unknown) > 0 {
-		return fmt.Errorf("unknown features: %s", strings.Join(unknown, ", "))
-	}
-	*fl = append(*fl, toAppend...)
-	return nil
-}
-
-// Delete removes given items from feature list while keeping its original
-// order.
-func (fl *FeatureList) Delete(items ...string) {
-	toDelete := FeatureList(items)
-	newList := []string{}
-	for _, item := range *fl {
-		if !toDelete.Has(item) {
-			newList = append(newList, item)
-		}
-	}
-	*fl = newList
-}
-
-// Has returns true if given feature exists in feature list. The check is
-// case-insensitive.
-func (fl FeatureList) Has(feature string) bool {
-	normalized, _ := NormalizeOpenShiftFeature(feature)
-	for _, item := range fl {
-		itemNormalized, _ := NormalizeOpenShiftFeature(item)
-		if normalized == itemNormalized {
-			return true
-		}
-	}
-	return false
-}
-
 // ParseNamespaceAndName returns back the namespace and name (empty if something goes wrong), for a given string.
 // This is useful when pointing to a particular resource inside of our config.
 func ParseNamespaceAndName(in string) (string, string, error) {

pkg/cmd/server/apis/config/types.go

@@ -9,10 +9,6 @@ import (
 
 // A new entry shall be added to FeatureAliases for every change to following values.
 const (
-	FeatureBuilder    = `Builder`
-	FeatureS2I        = `S2IBuilder`
-	FeatureWebConsole = `WebConsole`
-
 	AllVersions = "*"
 )
 
@@ -135,15 +131,6 @@ var (
 	KnownKubeAPIGroups   = sets.StringKeySet(KubeAPIGroupsToAllowedVersions)
 	KnownOriginAPIGroups = sets.StringKeySet(OriginAPIGroupsToAllowedVersions)
 
-	// FeatureAliases maps deprecated names of feature flag to their canonical
-	// names. Aliases must be lower-cased for O(1) lookup.
-	FeatureAliases = map[string]string{
-		"s2i builder": FeatureS2I,
-		"web console": FeatureWebConsole,
-	}
-	KnownOpenShiftFeatures = []string{FeatureBuilder, FeatureS2I, FeatureWebConsole}
-	AtomicDisabledFeatures = []string{FeatureBuilder, FeatureS2I, FeatureWebConsole}
-
 	// List public registries that we are allowing to import images from by default.
 	// By default all registries have set to be "secure", iow. the port for them is
 	// defaulted to "443".
@@ -330,8 +317,6 @@ const (
 	ControllersAll = "*"
 )
 
-type FeatureList []string
-
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 type MasterConfig struct {
@@ -379,9 +364,6 @@ type MasterConfig struct {
 
 	ControllerConfig ControllerConfig
 
-	// Allow to disable OpenShift components
-	DisabledFeatures FeatureList
-
 	// EtcdStorageConfig contains information about how API resources are
 	// stored in Etcd. These values are only relevant when etcd is the
 	// backing store for the cluster.