Merge pull request #33420 from thockin/openapi-make

Automatic merge from submit-queue

Fix make rebuilds for openapi

openapi generates a single file, so its rules can be significantly simpler.

federation generates an empty file which we previously elided, but which triggers deps every build.

This fixes both.

@mbohlool something to think about.  Generating a single file means you generate everything every time any tagged package changes.  This is not awesome.  In practice, it's pretty fast, so maybe just for thinking about - would it be better to generate a file for every tagged package, and only regenerate when needed.  The file could self-register a callback or just add some structs to your mega-map.

Makefile.generated_files

@@ -296,6 +296,7 @@ $(DEEPCOPY_GEN):
 # The result file, in each pkg, of open-api generation.
 OPENAPI_BASENAME := $(GENERATED_FILE_PREFIX)openapi
 OPENAPI_FILENAME := $(OPENAPI_BASENAME).go
+OPENAPI_OUTPUT_PKG := pkg/generated/openapi
 
 # The tool used to generate open apis.
 OPENAPI_GEN := $(BIN_DIR)/openapi-gen
@@ -310,19 +311,11 @@ OPENAPI_DIRS := $(shell                                             \
         | sort -u                                                   \
 )
 
-OPENAPI_FILES := $(addsuffix /$(OPENAPI_FILENAME), $(OPENAPI_DIRS))
+OPENAPI_OUTFILE := $(OPENAPI_OUTPUT_PKG)/$(OPENAPI_FILENAME)
 
-# This rule aggregates the set of files to generate and then generates them all
+# This rule is the user-friendly entrypoint for openapi generation.
-# in a single run of the tool.
 .PHONY: gen_openapi
-gen_openapi: $(OPENAPI_FILES)
+gen_openapi: $(OPENAPI_OUTFILE)
-	if [[ -f $(META_DIR)/$(OPENAPI_GEN).todo ]]; then                   \
-	    ./hack/run-in-gopath.sh $(OPENAPI_GEN)                          \
-	        --v $(KUBE_VERBOSE)                                         \
-	        --logtostderr                                               \
-	        -i $$(cat $(META_DIR)/$(OPENAPI_GEN).todo | paste -sd, -)   \
-	        -O $(OPENAPI_BASENAME);                                     \
-	fi
 
 # For each dir in OPENAPI_DIRS, this establishes a dependency between the
 # output file and the input files that should trigger a rebuild.
@@ -337,18 +330,18 @@ gen_openapi: $(OPENAPI_FILES)
 # We depend on the $(GOFILES_META).stamp to detect when the set of input files
 # has changed.  This allows us to detect deleted input files.
 $(foreach dir, $(OPENAPI_DIRS), $(eval                                     \
-    $(dir)/$(OPENAPI_FILENAME): $(META_DIR)/$(dir)/$(GOFILES_META).stamp   \
+    $(OPENAPI_OUTFILE): $(META_DIR)/$(dir)/$(GOFILES_META).stamp           \
                                  $(gofiles__$(dir))                        \
 ))
 
-# Unilaterally remove any leftovers from previous runs.
+# How to regenerate open-api code.  This emits a single file for all results.
-$(shell rm -f $(META_DIR)/$(OPENAPI_GEN)*.todo)
+$(OPENAPI_OUTFILE): $(OPENAPI_GEN)
-
+	./hack/run-in-gopath.sh $(OPENAPI_GEN)                                          \
-# How to regenerate open-api code.  We need to collect these up and trigger one
+	    --v $(KUBE_VERBOSE)                                                         \
-# single run to generate definition for all types.
+	    --logtostderr                                                               \
-$(OPENAPI_FILES): $(OPENAPI_GEN)
+	    -i $$(echo $(addprefix $(PRJ_SRC_PATH)/, $(OPENAPI_DIRS)) | sed 's/ /,/g')  \
-	mkdir -p $$(dirname $(META_DIR)/$(OPENAPI_GEN))
+	    -p $(PRJ_SRC_PATH)/$(OPENAPI_OUTPUT_PKG)                                    \
-	echo $(PRJ_SRC_PATH)/$(@D) >> $(META_DIR)/$(OPENAPI_GEN).todo
+	    -O $(OPENAPI_BASENAME)
 
 # This calculates the dependencies for the generator tool, so we only rebuild
 # it when needed.  It is PHONY so that it always runs, but it only updates the

cmd/libs/go2idl/openapi-gen/generators/openapi.go

@@ -30,7 +30,6 @@ import (
 	"k8s.io/gengo/namer"
 	"k8s.io/gengo/types"
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
-	"k8s.io/kubernetes/pkg/util/sets"
 
 	"github.com/golang/glog"
 )
@@ -42,8 +41,6 @@ const tagName = "k8s:openapi-gen"
 const (
 	tagValueTrue  = "true"
 	tagValueFalse = "false"
-	// Should only be used only for test
-	tagTargetType = "target"
 )
 
 func hasOpenAPITagValue(comments []string, value string) bool {
@@ -77,7 +74,6 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 	if err != nil {
 		glog.Fatalf("Failed loading boilerplate: %v", err)
 	}
-	inputs := sets.NewString(context.Inputs...)
 	header := append([]byte(fmt.Sprintf("// +build !%s\n\n", arguments.GeneratedBuildTag)), boilerplate...)
 	header = append(header, []byte(
 		`
@@ -85,32 +81,20 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 
 `)...)
 
-	targets := []*types.Package{}
+	if err := context.AddDir(arguments.OutputPackagePath); err != nil {
-	for i := range inputs {
+		glog.Fatalf("Failed to load output package: %v", err)
-		glog.V(5).Infof("considering pkg %q", i)
-		pkg, ok := context.Universe[i]
-		if !ok {
-			// If the input had no Go files, for example.
-			continue
-		}
-		if hasOpenAPITagValue(pkg.Comments, tagTargetType) || hasOpenAPITagValue(pkg.DocComments, tagTargetType) {
-			glog.V(5).Infof("target package : %q", pkg)
-			targets = append(targets, pkg)
-		}
 	}
-	switch len(targets) {
+	pkg := context.Universe[arguments.OutputPackagePath]
-	case 0:
+	if pkg == nil {
-		// If no target package found, that means the generated file in target package is up to date
+		glog.Fatalf("Got nil output package: %v", err)
-		// and build excluded the target package.
+	}
-		return generator.Packages{}
+	return generator.Packages{
-	case 1:
+		&generator.DefaultPackage{
-		pkg := targets[0]
-		return generator.Packages{&generator.DefaultPackage{
 			PackageName: strings.Split(filepath.Base(pkg.Path), ".")[0],
 			PackagePath: pkg.Path,
 			HeaderText:  header,
 			GeneratorFunc: func(c *generator.Context) (generators []generator.Generator) {
-				return []generator.Generator{NewOpenAPIGen(arguments.OutputFileBaseName, targets[0], context)}
+				return []generator.Generator{NewOpenAPIGen(arguments.OutputFileBaseName, pkg, context)}
 			},
 			FilterFunc: func(c *generator.Context, t *types.Type) bool {
 				// There is a conflict between this codegen and codecgen, we should avoid types generated for codecgen
@@ -127,11 +111,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 				return false
 			},
 		},
-		}
-	default:
-		glog.Fatalf("Duplicate target type found: %v", targets)
 	}
-	return generator.Packages{}
 }
 
 const (

pkg/generated/openapi/doc.go

@@ -15,5 +15,4 @@ limitations under the License.
 */
 
 // openapi generated definitions.
-// +k8s:openapi-gen=target
 package openapi