Review feedback

pkg/apis/serving/v1alpha1/configuration_types.go

@@ -210,9 +210,10 @@ func (cs *ConfigurationStatus) RemoveCondition(t ConfigurationConditionType) {
 }
 
 func (cs *ConfigurationStatus) InitializeConditions() {
-	rct := []ConfigurationConditionType{
-		ConfigurationConditionLatestRevisionReady, ConfigurationConditionReady}
-	for _, cond := range rct {
+	for _, cond := range []ConfigurationConditionType{
+		ConfigurationConditionLatestRevisionReady,
+		ConfigurationConditionReady,
+	} {
 		if rc := cs.GetCondition(cond); rc == nil {
 			cs.setCondition(&ConfigurationCondition{
 				Type:   cond,
@@ -235,7 +236,9 @@ func (cs *ConfigurationStatus) SetLatestCreatedRevisionName(name string) {
 func (cs *ConfigurationStatus) SetLatestReadyRevisionName(name string) {
 	cs.LatestReadyRevisionName = name
 	for _, cond := range []ConfigurationConditionType{
-		ConfigurationConditionLatestRevisionReady, ConfigurationConditionReady} {
+		ConfigurationConditionLatestRevisionReady,
+		ConfigurationConditionReady,
+	} {
 		cs.setCondition(&ConfigurationCondition{
 			Type:   cond,
 			Status: corev1.ConditionTrue,

pkg/apis/serving/v1alpha1/revision_types.go

@@ -273,9 +273,11 @@ func (rs *RevisionStatus) RemoveCondition(t RevisionConditionType) {
 func (rs *RevisionStatus) InitializeConditions() {
 	// We don't include BuildSucceeded here because it could confuse users if
 	// no `buildName` was specified.
-	rct := []RevisionConditionType{RevisionConditionResourcesAvailable,
-		RevisionConditionContainerHealthy, RevisionConditionReady}
-	for _, cond := range rct {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionResourcesAvailable,
+		RevisionConditionContainerHealthy,
+		RevisionConditionReady,
+	} {
 		if rc := rs.GetCondition(cond); rc == nil {
 			rs.setCondition(&RevisionCondition{
 				Type:   cond,
@@ -295,7 +297,10 @@ func (rs *RevisionStatus) InitializeBuildCondition() {
 }
 
 func (rs *RevisionStatus) MarkBuilding() {
-	for _, cond := range []RevisionConditionType{RevisionConditionBuildSucceeded, RevisionConditionReady} {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionBuildSucceeded,
+		RevisionConditionReady,
+	} {
 		rs.setCondition(&RevisionCondition{
 			Type:   cond,
 			Status: corev1.ConditionUnknown,
@@ -318,7 +323,10 @@ func (rs *RevisionStatus) MarkBuildSucceeded() {
 }
 
 func (rs *RevisionStatus) MarkBuildFailed(bc *buildv1alpha1.BuildCondition) {
-	for _, cond := range []RevisionConditionType{RevisionConditionBuildSucceeded, RevisionConditionReady} {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionBuildSucceeded,
+		RevisionConditionReady,
+	} {
 		rs.setCondition(&RevisionCondition{
 			Type:    cond,
 			Status:  corev1.ConditionFalse,
@@ -329,9 +337,11 @@ func (rs *RevisionStatus) MarkBuildFailed(bc *buildv1alpha1.BuildCondition) {
 }
 
 func (rs *RevisionStatus) MarkDeploying(reason string) {
-	rct := []RevisionConditionType{RevisionConditionResourcesAvailable,
-		RevisionConditionContainerHealthy, RevisionConditionReady}
-	for _, cond := range rct {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionResourcesAvailable,
+		RevisionConditionContainerHealthy,
+		RevisionConditionReady,
+	} {
 		rs.setCondition(&RevisionCondition{
 			Type:   cond,
 			Status: corev1.ConditionUnknown,
@@ -341,7 +351,10 @@ func (rs *RevisionStatus) MarkDeploying(reason string) {
 }
 
 func (rs *RevisionStatus) MarkServiceTimeout() {
-	for _, cond := range []RevisionConditionType{RevisionConditionResourcesAvailable, RevisionConditionReady} {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionResourcesAvailable,
+		RevisionConditionReady,
+	} {
 		rs.setCondition(&RevisionCondition{
 			Type:    cond,
 			Status:  corev1.ConditionFalse,
@@ -352,7 +365,10 @@ func (rs *RevisionStatus) MarkServiceTimeout() {
 }
 
 func (rs *RevisionStatus) MarkProgressDeadlineExceeded(message string) {
-	for _, cond := range []RevisionConditionType{RevisionConditionResourcesAvailable, RevisionConditionReady} {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionResourcesAvailable,
+		RevisionConditionReady,
+	} {
 		rs.setCondition(&RevisionCondition{
 			Type:    cond,
 			Status:  corev1.ConditionFalse,
@@ -387,7 +403,10 @@ func (rs *RevisionStatus) MarkInactive() {
 }
 
 func (rs *RevisionStatus) MarkContainerMissing(message string) {
-	for _, cond := range []RevisionConditionType{RevisionConditionContainerHealthy, RevisionConditionReady} {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionContainerHealthy,
+		RevisionConditionReady,
+	} {
 		rs.setCondition(&RevisionCondition{
 			Type:    cond,
 			Status:  corev1.ConditionFalse,
@@ -398,8 +417,10 @@ func (rs *RevisionStatus) MarkContainerMissing(message string) {
 }
 
 func (rs *RevisionStatus) checkAndMarkReady() {
-	rct := []RevisionConditionType{RevisionConditionContainerHealthy, RevisionConditionResourcesAvailable}
-	for _, cond := range rct {
+	for _, cond := range []RevisionConditionType{
+		RevisionConditionContainerHealthy,
+		RevisionConditionResourcesAvailable,
+	} {
 		c := rs.GetCondition(cond)
 		if c == nil || c.Status != corev1.ConditionTrue {
 			return

pkg/apis/serving/v1alpha1/route_types.go

@@ -221,9 +221,11 @@ func (rs *RouteStatus) RemoveCondition(t RouteConditionType) {
 }
 
 func (rs *RouteStatus) InitializeConditions() {
-	rct := []RouteConditionType{RouteConditionAllTrafficAssigned,
-		RouteConditionIngressReady, RouteConditionReady}
-	for _, cond := range rct {
+	for _, cond := range []RouteConditionType{
+		RouteConditionAllTrafficAssigned,
+		RouteConditionIngressReady,
+		RouteConditionReady,
+	} {
 		if rc := rs.GetCondition(cond); rc == nil {
 			rs.setCondition(&RouteCondition{
 				Type:   cond,
@@ -242,7 +244,10 @@ func (rs *RouteStatus) MarkTrafficAssigned() {
 }
 
 func (rs *RouteStatus) MarkTrafficNotAssigned(kind, name string) {
-	for _, cond := range []RouteConditionType{RouteConditionAllTrafficAssigned, RouteConditionReady} {
+	for _, cond := range []RouteConditionType{
+		RouteConditionAllTrafficAssigned,
+		RouteConditionReady,
+	} {
 		rs.setCondition(&RouteCondition{
 			Type:    cond,
 			Status:  corev1.ConditionFalse,
@@ -261,7 +266,10 @@ func (rs *RouteStatus) MarkIngressReady() {
 }
 
 func (rs *RouteStatus) checkAndMarkReady() {
-	for _, cond := range []RouteConditionType{RouteConditionAllTrafficAssigned, RouteConditionIngressReady} {
+	for _, cond := range []RouteConditionType{
+		RouteConditionAllTrafficAssigned,
+		RouteConditionIngressReady,
+	} {
 		ata := rs.GetCondition(cond)
 		if ata == nil || ata.Status != corev1.ConditionTrue {
 			return

pkg/apis/serving/v1alpha1/service_types.go

@@ -189,9 +189,11 @@ func (ss *ServiceStatus) RemoveCondition(t ServiceConditionType) {
 }
 
 func (ss *ServiceStatus) InitializeConditions() {
-	sct := []ServiceConditionType{ServiceConditionReady,
-		ServiceConditionConfigurationReady, ServiceConditionRouteReady}
-	for _, cond := range sct {
+	for _, cond := range []ServiceConditionType{
+		ServiceConditionReady,
+		ServiceConditionConfigurationReady,
+		ServiceConditionRouteReady,
+	} {
 		if rc := ss.GetCondition(cond); rc == nil {
 			ss.setCondition(&ServiceCondition{
 				Type:   cond,
@@ -201,7 +203,7 @@ func (ss *ServiceStatus) InitializeConditions() {
 	}
 }
 
-func (ss *ServiceStatus) PropagateConfiguration(cs ConfigurationStatus) {
+func (ss *ServiceStatus) PropagateConfigurationStatus(cs ConfigurationStatus) {
 	cc := cs.GetCondition(ConfigurationConditionReady)
 	if cc == nil {
 		return
@@ -224,7 +226,7 @@ func (ss *ServiceStatus) PropagateConfiguration(cs ConfigurationStatus) {
 	}
 }
 
-func (ss *ServiceStatus) PropagateRoute(rs RouteStatus) {
+func (ss *ServiceStatus) PropagateRouteStatus(rs RouteStatus) {
 	rc := rs.GetCondition(RouteConditionReady)
 	if rc == nil {
 		return
@@ -248,8 +250,10 @@ func (ss *ServiceStatus) PropagateRoute(rs RouteStatus) {
 }
 
 func (ss *ServiceStatus) checkAndMarkReady() {
-	sct := []ServiceConditionType{ServiceConditionConfigurationReady, ServiceConditionRouteReady}
-	for _, cond := range sct {
+	for _, cond := range []ServiceConditionType{
+		ServiceConditionConfigurationReady,
+		ServiceConditionRouteReady,
+	} {
 		c := ss.GetCondition(cond)
 		if c == nil || c.Status != corev1.ConditionTrue {
 			return

pkg/apis/serving/v1alpha1/service_types_test.go

@@ -192,19 +192,19 @@ func TestTypicalServiceFlow(t *testing.T) {
 	checkConditionOngoingService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Nothing from Configuration is nothing to us.
-	svc.Status.PropagateConfiguration(ConfigurationStatus{})
+	svc.Status.PropagateConfigurationStatus(ConfigurationStatus{})
 	checkConditionOngoingService(svc.Status, ServiceConditionReady, t)
 	checkConditionOngoingService(svc.Status, ServiceConditionConfigurationReady, t)
 	checkConditionOngoingService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Nothing from Route is nothing to us.
-	svc.Status.PropagateRoute(RouteStatus{})
+	svc.Status.PropagateRouteStatus(RouteStatus{})
 	checkConditionOngoingService(svc.Status, ServiceConditionReady, t)
 	checkConditionOngoingService(svc.Status, ServiceConditionConfigurationReady, t)
 	checkConditionOngoingService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Done from Configuration moves our ConfigurationReady condition
-	svc.Status.PropagateConfiguration(ConfigurationStatus{
+	svc.Status.PropagateConfigurationStatus(ConfigurationStatus{
 		Conditions: []ConfigurationCondition{{
 			Type:   ConfigurationConditionReady,
 			Status: corev1.ConditionTrue,
@@ -215,7 +215,7 @@ func TestTypicalServiceFlow(t *testing.T) {
 	checkConditionOngoingService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Done from Route moves our RouteReady condition, which triggers us to be Ready.
-	svc.Status.PropagateRoute(RouteStatus{
+	svc.Status.PropagateRouteStatus(RouteStatus{
 		Conditions: []RouteCondition{{
 			Type:   RouteConditionReady,
 			Status: corev1.ConditionTrue,
@@ -226,7 +226,7 @@ func TestTypicalServiceFlow(t *testing.T) {
 	checkConditionSucceededService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Check idempotency
-	svc.Status.PropagateRoute(RouteStatus{
+	svc.Status.PropagateRouteStatus(RouteStatus{
 		Conditions: []RouteCondition{{
 			Type:   RouteConditionReady,
 			Status: corev1.ConditionTrue,
@@ -237,7 +237,7 @@ func TestTypicalServiceFlow(t *testing.T) {
 	checkConditionSucceededService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Failure causes us to become unready immediately (config still ok).
-	svc.Status.PropagateRoute(RouteStatus{
+	svc.Status.PropagateRouteStatus(RouteStatus{
 		Conditions: []RouteCondition{{
 			Type:   RouteConditionReady,
 			Status: corev1.ConditionFalse,
@@ -248,7 +248,7 @@ func TestTypicalServiceFlow(t *testing.T) {
 	checkConditionFailedService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Fixed the glitch.
-	svc.Status.PropagateRoute(RouteStatus{
+	svc.Status.PropagateRouteStatus(RouteStatus{
 		Conditions: []RouteCondition{{
 			Type:   RouteConditionReady,
 			Status: corev1.ConditionTrue,
@@ -267,7 +267,7 @@ func TestConfigurationFailurePropagation(t *testing.T) {
 	checkConditionOngoingService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Failure causes us to become unready immediately
-	svc.Status.PropagateConfiguration(ConfigurationStatus{
+	svc.Status.PropagateConfigurationStatus(ConfigurationStatus{
 		Conditions: []ConfigurationCondition{{
 			Type:   ConfigurationConditionReady,
 			Status: corev1.ConditionFalse,
@@ -286,7 +286,7 @@ func TestRouteFailurePropagation(t *testing.T) {
 	checkConditionOngoingService(svc.Status, ServiceConditionRouteReady, t)
 
 	// Failure causes us to become unready immediately
-	svc.Status.PropagateRoute(RouteStatus{
+	svc.Status.PropagateRouteStatus(RouteStatus{
 		Conditions: []RouteCondition{{
 			Type:   RouteConditionReady,
 			Status: corev1.ConditionFalse,

pkg/controller/configuration/configuration.go

@@ -45,8 +45,8 @@ type Controller struct {
 	buildClientSet buildclientset.Interface
 
 	// listers index properties about resources
-	lister         listers.ConfigurationLister
-	revisionLister listers.RevisionLister
+	configurationLister listers.ConfigurationLister
+	revisionLister      listers.RevisionLister
 }
 
 // NewController creates a new Configuration controller
@@ -58,20 +58,23 @@ func NewController(
 
 	// obtain references to a shared index informer for the Configuration
 	// and Revision type.
-	informer := elaInformerFactory.Serving().V1alpha1().Configurations()
+	configurationInformer := elaInformerFactory.Serving().V1alpha1().Configurations()
 	revisionInformer := elaInformerFactory.Serving().V1alpha1().Revisions()
 
-	informers := []cache.SharedIndexInformer{informer.Informer(), revisionInformer.Informer()}
+	informers := []cache.SharedIndexInformer{
+		configurationInformer.Informer(),
+		revisionInformer.Informer(),
+	}
 
 	c := &Controller{
-		Base:           controller.NewBase(opt, controllerAgentName, "Configurations", informers),
-		buildClientSet: buildClientSet,
-		lister:         informer.Lister(),
-		revisionLister: revisionInformer.Lister(),
+		Base:                controller.NewBase(opt, controllerAgentName, "Configurations", informers),
+		buildClientSet:      buildClientSet,
+		configurationLister: configurationInformer.Lister(),
+		revisionLister:      revisionInformer.Lister(),
 	}
 
 	c.Logger.Info("Setting up event handlers")
-	informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+	configurationInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc:    c.Enqueue,
 		UpdateFunc: controller.PassNew(c.Enqueue),
 		DeleteFunc: c.Enqueue,
@@ -114,7 +117,7 @@ func (c *Controller) Reconcile(key string) error {
 	logger := loggerWithConfigInfo(c.Logger, namespace, name)
 
 	// Get the Configuration resource with this namespace/name
-	config, err := c.lister.Configurations(namespace).Get(name)
+	config, err := c.configurationLister.Configurations(namespace).Get(name)
 	if errors.IsNotFound(err) {
 		// The resource no longer exists, in which case we stop processing.
 		runtime.HandleError(fmt.Errorf("configuration %q in work queue no longer exists", key))
@@ -249,7 +252,7 @@ func generateRevisionName(u *v1alpha1.Configuration) string {
 }
 
 func (c *Controller) updateStatus(u *v1alpha1.Configuration) (*v1alpha1.Configuration, error) {
-	newu, err := c.lister.Configurations(u.Namespace).Get(u.Name)
+	newu, err := c.configurationLister.Configurations(u.Namespace).Get(u.Name)
 	if err != nil {
 		return nil, err
 	}