Merge pull request #2101 from GoogleCloudPlatform/release-candidate

Release v1.27.0

.codespellrc

@@ -0,0 +1,7 @@
+[codespell]
+# js/ folder seems to contain vendored code
+skip = .git,*.pdf,*.svg,*.min.js,*.js.map,js,requirements.txt
+# docs/videos/healthcare-and-life-sciences/lysozyme-example/submit.sh has CONECT - might be "legit"
+ignore-regex = HETATM -e CONECT|\([A-Z]\)[a-z]+
+#
+ignore-words-list = namd,tempdate,te,ue,startd,passtime

.github/workflows/pr-label-validation.yml

@@ -43,6 +43,13 @@ jobs:
         count: 1
         labels: "release-chore, release-key-new-features, release-new-modules, release-module-improvements, release-improvements, release-deprecations, release-version-updates, release-bugfix"
         message: "This PR is being prevented from merging because it is not labeled.  Please add a label to this PR.  Accepted labels: release-chore, release-key-new-features, release-new-modules, release-module-improvements, release-improvements, release-deprecations, release-version-updates, release-bugfix"
+    - id: do-not-merge
+      uses: mheap/github-action-required-labels@v5
+      with:
+        mode: exactly
+        count: 0
+        labels: "do-not-merge"
+        add_comment: false
     - id: print-labels
       run: |
         echo "Current PR labels:"

.gitignore

@@ -63,6 +63,9 @@ packer-manifest.json
 #### Python Virtual Environments
 venv*/
 
+### Python cache files
+**/__pycache__
+
 #### Exclude from gitingore
 !tools/validate_configs/golden_copies/expectations/*/*/*/defaults.auto.pkrvars.hcl
 !tools/validate_configs/golden_copies/expectations/*/*/terraform.tfvars

.pre-commit-config.yaml

@@ -39,6 +39,7 @@ repos:
     entry: tools/autodoc/terraform_docs.sh
     language: script
     files: ^.*\.pkr\.hcl$
+    exclude: (pkg\/.*$)|(tools/validate_configs/golden_copies/.*)
     pass_filenames: true
     require_serial: true
   - id: duplicate-diff
@@ -57,22 +58,29 @@ repos:
     files: '.*\.tf$'
     pass_filenames: false
     require_serial: true
+  - id: addlicense
+    name: addlicense
+    language: system
+    entry: addlicense
+    args: ['-c', '"Google LLC"', '-l', 'apache']
+    exclude: docs/videos/healthcare-and-life-sciences/lysozyme-example/submit.sh
+    pass_filenames: true
+
 - repo: https://github.com/dnephin/pre-commit-golang
   rev: v0.5.1
   hooks:
   - id: go-fmt
   - id: go-vet
   - id: go-imports
   - id: go-cyclo
-    args: [-over=15]
+    args: [-over=13]
   - id: go-unit-tests
-  - id: go-build
   - id: go-mod-tidy
-- repo: https://github.com/tekwizely/pre-commit-golang
-  rev: v1.0.0-rc.1
-  hooks:
-  - id: go-critic
-    args: [-disable, "#experimental,sloppyTypeAssert"]
+# Disabled temporarily due to https://github.com/go-critic/go-critic/issues/1388
+# - repo: https://github.com/tekwizely/pre-commit-golang
+#   rev: v1.0.0-rc.1
+#   hooks:
+#   - id: go-critic
 - repo: https://github.com/Bahjat/pre-commit-golang
   rev: v1.0.2
   hooks:
@@ -105,4 +113,9 @@ repos:
   rev: v4.4.0
   hooks:
   - id: end-of-file-fixer
+- repo: https://github.com/codespell-project/codespell
+  rev: v2.2.6
+  hooks:
+  - id: codespell
+    exclude: requirements.txt$|/js/|go.sum$
 exclude: tools/validate_configs/golden_copies/.*

.tflint.hcl

@@ -11,6 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 plugin "google" {
   enabled = true
   version = "0.26.0"

Makefile

@@ -3,7 +3,7 @@ MIN_PACKER_VERSION=1.7.9 # for building images
 MIN_TERRAFORM_VERSION=1.2 # for deploying modules
 MIN_GOLANG_VERSION=1.18 # for building ghpc
 
-.PHONY: install install-user tests format add-google-license install-dev-deps \
+.PHONY: install install-user tests format install-dev-deps \
         warn-go-missing warn-terraform-missing warn-packer-missing \
         warn-go-version warn-terraform-version warn-packer-version \
         test-engine validate_configs validate_golden_copy packer-check \
@@ -65,15 +65,6 @@ install-dev-deps: warn-terraform-version warn-packer-version check-pre-commit ch
 	go install golang.org/x/tools/cmd/goimports@latest
 	go install honnef.co/go/tools/cmd/staticcheck@latest
 
-ifeq (, $(shell which addlicense))
-add-google-license:
-	$(error "could not find addlicense in PATH, run: go install github.com/google/addlicense@latest")
-else
-add-google-license:
-	# lysozyme-example is under CC-BY-4.0
-	addlicense -c "Google LLC" -l apache -ignore **/lysozyme-example/submit.sh .
-endif
-
 # RULES SUPPORTING THE ABOVE
 
 test-engine: warn-go-missing

cmd/create.go

@@ -21,9 +21,10 @@ import (
 	"errors"
 	"fmt"
 	"hpc-toolkit/pkg/config"
+	"hpc-toolkit/pkg/logging"
 	"hpc-toolkit/pkg/modulewriter"
 	"hpc-toolkit/pkg/validators"
-	"log"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -78,51 +79,47 @@ var (
 
 func runCreateCmd(cmd *cobra.Command, args []string) {
 	dc := expandOrDie(args[0])
-	deplName, err := dc.Config.DeploymentName()
-	checkErr(err)
-	deplDir := filepath.Join(outputDir, deplName)
-	checkErr(modulewriter.WriteDeployment(dc, deplDir, overwriteDeployment))
-
-	fmt.Println("To deploy your infrastructure please run:")
-	fmt.Println()
-	fmt.Printf(boldGreen("%s deploy %s\n"), execPath(), deplDir)
-	fmt.Println()
+	deplDir := filepath.Join(outputDir, dc.Config.DeploymentName())
+	checkErr(checkOverwriteAllowed(deplDir, dc.Config, overwriteDeployment))
+	checkErr(modulewriter.WriteDeployment(dc, deplDir))
+
+	logging.Info("To deploy your infrastructure please run:")
+	logging.Info("")
+	logging.Info(boldGreen("%s deploy %s"), execPath(), deplDir)
+	logging.Info("")
 	printAdvancedInstructionsMessage(deplDir)
 }
 
 func printAdvancedInstructionsMessage(deplDir string) {
-	fmt.Println("Find instructions for cleanly destroying infrastructure and advanced manual")
-	fmt.Println("deployment instructions at:")
-	fmt.Println()
-	fmt.Printf("%s\n", modulewriter.InstructionsPath(deplDir))
+	logging.Info("Find instructions for cleanly destroying infrastructure and advanced manual")
+	logging.Info("deployment instructions at:")
+	logging.Info("")
+	logging.Info(modulewriter.InstructionsPath(deplDir))
 }
 
 func expandOrDie(path string) config.DeploymentConfig {
 	dc, ctx, err := config.NewDeploymentConfig(path)
 	if err != nil {
-		log.Fatal(renderError(err, ctx))
+		logging.Fatal(renderError(err, ctx))
 	}
 	// Set properties from CLI
 	if err := setCLIVariables(&dc.Config, cliVariables); err != nil {
-		log.Fatalf("Failed to set the variables at CLI: %v", err)
+		logging.Fatal("Failed to set the variables at CLI: %v", err)
 	}
 	if err := setBackendConfig(&dc.Config, cliBEConfigVars); err != nil {
-		log.Fatalf("Failed to set the backend config at CLI: %v", err)
-	}
-	if err := setValidationLevel(&dc.Config, validationLevel); err != nil {
-		log.Fatal(err)
-	}
-	if err := skipValidators(&dc); err != nil {
-		log.Fatal(err)
+		logging.Fatal("Failed to set the backend config at CLI: %v", err)
 	}
+	checkErr(setValidationLevel(&dc.Config, validationLevel))
+	checkErr(skipValidators(&dc))
+
 	if dc.Config.GhpcVersion != "" {
-		fmt.Printf("ghpc_version setting is ignored.")
+		logging.Info("ghpc_version setting is ignored.")
 	}
 	dc.Config.GhpcVersion = GitCommitInfo
 
 	// Expand the blueprint
 	if err := dc.ExpandConfig(); err != nil {
-		log.Fatal(renderError(err, ctx))
+		logging.Fatal(renderError(err, ctx))
 	}
 
 	validateMaybeDie(dc.Config, ctx)
@@ -134,29 +131,29 @@ func validateMaybeDie(bp config.Blueprint, ctx config.YamlCtx) {
 	if err == nil {
 		return
 	}
-	log.Println(renderError(err, ctx))
-
-	log.Println("One or more blueprint validators has failed. See messages above for suggested")
-	log.Println("actions. General troubleshooting guidance and instructions for configuring")
-	log.Println("validators are shown below.")
-	log.Println("")
-	log.Println("- https://goo.gle/hpc-toolkit-troubleshooting")
-	log.Println("- https://goo.gle/hpc-toolkit-validation")
-	log.Println("")
-	log.Println("Validators can be silenced or treated as warnings or errors:")
-	log.Println("")
-	log.Println("- https://goo.gle/hpc-toolkit-validation-levels")
-	log.Println("")
+	logging.Error(renderError(err, ctx))
+
+	logging.Error("One or more blueprint validators has failed. See messages above for suggested")
+	logging.Error("actions. General troubleshooting guidance and instructions for configuring")
+	logging.Error("validators are shown below.")
+	logging.Error("")
+	logging.Error("- https://goo.gle/hpc-toolkit-troubleshooting")
+	logging.Error("- https://goo.gle/hpc-toolkit-validation")
+	logging.Error("")
+	logging.Error("Validators can be silenced or treated as warnings or errors:")
+	logging.Error("")
+	logging.Error("- https://goo.gle/hpc-toolkit-validation-levels")
+	logging.Error("")
 
 	switch bp.ValidationLevel {
 	case config.ValidationWarning:
 		{
-			log.Println(boldYellow("Validation failures were treated as a warning, continuing to create blueprint."))
-			log.Println("")
+			logging.Error(boldYellow("Validation failures were treated as a warning, continuing to create blueprint."))
+			logging.Error("")
 		}
 	case config.ValidationError:
 		{
-			log.Fatal(boldRed("validation failed due to the issues listed above"))
+			logging.Fatal(boldRed("validation failed due to the issues listed above"))
 		}
 	}
 
@@ -289,3 +286,46 @@ func filterYaml(cmd *cobra.Command, args []string, toComplete string) ([]string,
 	}
 	return []string{"yaml", "yml"}, cobra.ShellCompDirectiveFilterFileExt
 }
+
+// Determines if overwrite is allowed
+func checkOverwriteAllowed(depDir string, bp config.Blueprint, overwriteFlag bool) error {
+	if _, err := os.Stat(depDir); os.IsNotExist(err) {
+		return nil // all good, no previous deployment
+	}
+
+	if _, err := os.Stat(modulewriter.HiddenGhpcDir(depDir)); os.IsNotExist(err) {
+		// hidden ghpc dir does not exist
+		return fmt.Errorf("folder %q already exists, and it is not a valid GHPC deployment folder", depDir)
+	}
+
+	// try to get previous deployment
+	expPath := filepath.Join(modulewriter.ArtifactsDir(depDir), modulewriter.ExpandedBlueprintName)
+	if _, err := os.Stat(expPath); os.IsNotExist(err) {
+		return fmt.Errorf("expanded blueprint file %q is missing, this could be a result of changing GHPC version between consecutive deployments", expPath)
+	}
+	prev, _, err := config.NewDeploymentConfig(expPath)
+	if err != nil {
+		return err
+	}
+
+	if prev.Config.GhpcVersion != bp.GhpcVersion {
+		logging.Info("WARNING: ghpc_version has changed from %q to %q, using different versions of GHPC to update a live deployment is not officially supported. Proceed at your own risk", prev.Config.GhpcVersion, bp.GhpcVersion)
+	}
+
+	if !overwriteFlag {
+		return fmt.Errorf("deployment folder %q already exists, use -w to overwrite", depDir)
+	}
+
+	newGroups := map[config.GroupName]bool{}
+	for _, g := range bp.DeploymentGroups {
+		newGroups[g.Name] = true
+	}
+
+	for _, g := range prev.Config.DeploymentGroups {
+		if !newGroups[g.Name] {
+			return fmt.Errorf("you are attempting to remove a deployment group %q, which is not supported", g.Name)
+		}
+	}
+
+	return nil
+}

cmd/create_test.go

@@ -17,6 +17,9 @@ package cmd
 import (
 	"errors"
 	"hpc-toolkit/pkg/config"
+	"hpc-toolkit/pkg/modulewriter"
+	"os"
+	"path/filepath"
 
 	"github.com/zclconf/go-cty/cty"
 	. "gopkg.in/check.v1"
@@ -164,3 +167,76 @@ func (s *MySuite) TestValidateMaybeDie(c *C) {
 	ctx, _ := config.NewYamlCtx([]byte{})
 	validateMaybeDie(bp, ctx) // smoke test
 }
+
+func (s *MySuite) TestIsOverwriteAllowed_Absent(c *C) {
+	testDir := c.MkDir()
+	depDir := filepath.Join(testDir, "casper")
+
+	bp := config.Blueprint{}
+	c.Check(checkOverwriteAllowed(depDir, bp, false /*overwriteFlag*/), IsNil)
+	c.Check(checkOverwriteAllowed(depDir, bp, true /*overwriteFlag*/), IsNil)
+}
+
+func (s *MySuite) TestIsOverwriteAllowed_NotGHPC(c *C) {
+	depDir := c.MkDir() // empty deployment folder considered malformed
+
+	bp := config.Blueprint{}
+	c.Check(checkOverwriteAllowed(depDir, bp, false /*overwriteFlag*/), ErrorMatches, ".* not a valid GHPC deployment folder")
+	c.Check(checkOverwriteAllowed(depDir, bp, true /*overwriteFlag*/), ErrorMatches, ".* not a valid GHPC deployment folder")
+}
+
+func (s *MySuite) TestIsOverwriteAllowed_NoExpanded(c *C) {
+	depDir := c.MkDir() // empty deployment folder considered malformed
+	if err := os.MkdirAll(modulewriter.HiddenGhpcDir(depDir), 0755); err != nil {
+		c.Fatal(err)
+	}
+
+	bp := config.Blueprint{}
+	c.Check(checkOverwriteAllowed(depDir, bp, false /*overwriteFlag*/), ErrorMatches, ".* changing GHPC version.*")
+	c.Check(checkOverwriteAllowed(depDir, bp, true /*overwriteFlag*/), ErrorMatches, ".* changing GHPC version.*")
+}
+
+func (s *MySuite) TestIsOverwriteAllowed_Malformed(c *C) {
+	depDir := c.MkDir() // empty deployment folder considered malformed
+	if err := os.MkdirAll(modulewriter.ArtifactsDir(depDir), 0755); err != nil {
+		c.Fatal(err)
+	}
+	expPath := filepath.Join(modulewriter.ArtifactsDir(depDir), "expanded_blueprint.yaml")
+	if err := os.WriteFile(expPath, []byte("humus"), 0644); err != nil {
+		c.Fatal(err)
+	}
+
+	bp := config.Blueprint{}
+	c.Check(checkOverwriteAllowed(depDir, bp, false /*overwriteFlag*/), NotNil)
+	c.Check(checkOverwriteAllowed(depDir, bp, true /*overwriteFlag*/), NotNil)
+}
+
+func (s *MySuite) TestIsOverwriteAllowed_Present(c *C) {
+	depDir := c.MkDir()
+	artDir := modulewriter.ArtifactsDir(depDir)
+	if err := os.MkdirAll(artDir, 0755); err != nil {
+		c.Fatal(err)
+	}
+
+	prev := config.DeploymentConfig{
+		Config: config.Blueprint{
+			GhpcVersion: "TaleOdBygoneYears",
+			DeploymentGroups: []config.DeploymentGroup{
+				{Name: "isildur"}}}}
+	if err := prev.ExportBlueprint(filepath.Join(artDir, "expanded_blueprint.yaml")); err != nil {
+		c.Fatal(err)
+	}
+
+	super := config.Blueprint{
+		DeploymentGroups: []config.DeploymentGroup{
+			{Name: "isildur"},
+			{Name: "elendil"}}}
+	c.Check(checkOverwriteAllowed(depDir, super, false /*overwriteFlag*/), ErrorMatches, ".* already exists, use -w to overwrite")
+	c.Check(checkOverwriteAllowed(depDir, super, true /*overwriteFlag*/), IsNil)
+
+	sub := config.Blueprint{
+		DeploymentGroups: []config.DeploymentGroup{
+			{Name: "aragorn"}}}
+	c.Check(checkOverwriteAllowed(depDir, sub, false /*overwriteFlag*/), ErrorMatches, `.* already exists, use -w to overwrite`)
+	c.Check(checkOverwriteAllowed(depDir, sub, true /*overwriteFlag*/), ErrorMatches, `.*remove a deployment group "isildur".*`)
+}