Use gcloud builds submit rather than gcloud container builds submit (#…

…1069)

* Use gcloud builds submit rather than gcloud container builds submit for gcloud versions >= 207.0.0

GoogleCloudExtension/GoogleCloudExtension.GCloud/Models/CloudSdkVersions.cs

@@ -13,6 +13,8 @@
 // limitations under the License.
 
 using Newtonsoft.Json;
+using Newtonsoft.Json.Converters;
+using System;
 
 namespace GoogleCloudExtension.GCloud.Models
 {
@@ -25,6 +27,7 @@ public class CloudSdkVersions
         /// The version of the Cloud SDK itself.
         /// </summary>
         [JsonProperty("Google Cloud SDK")]
-        public string SdkVersion { get; set; }
+        [JsonConverter(typeof(VersionConverter))]
+        public Version SdkVersion { get; set; }
     }
 }

GoogleCloudExtension/GoogleCloudExtension/GCloud/GCloudContext.cs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 using GoogleCloudExtension.Accounts;
+using GoogleCloudExtension.GCloud.Models;
 using GoogleCloudExtension.Utils;
 using System;
 using System.Collections.Generic;
@@ -26,34 +27,48 @@ namespace GoogleCloudExtension.GCloud
     /// </summary>
     public class GCloudContext : IGCloudContext
     {
+        /// <summary>
+        /// The first version of gcloud with the builds group.
+        /// </summary>
+        /// <seealso href="https://cloud.google.com/sdk/docs/release-notes#20700_2018-06-26"/>
+        public const string GCloudBuildsMinimumVersion = "207.0.0";
+
         private const string GCloudMetricsVariable = "CLOUDSDK_METRICS_ENVIRONMENT";
         private const string GCloudMetricsVersionVariable = "CLOUDSDK_METRICS_ENVIRONMENT_VERSION";
 
+        /// <summary>
+        /// The first version of gcloud with the builds group.
+        /// </summary>
+        /// <see cref="GCloudBuildsMinimumVersion"/>
+        private static readonly Version s_gCloudBuildsMinimumVersion = new Version(GCloudBuildsMinimumVersion);
+
         /// <summary>
         /// The path to the credentials .json file to use for the call. The .json file should be a
-        /// format accetable by gcloud's --credential-file-override parameter. Typically an authorize_user kind.
+        /// format acceptable by gcloud's --credential-file-override parameter. Typically an authorize_user kind.
         /// </summary>
         public string CredentialsPath { get; }
 
         /// <summary>
-        /// The project id of the project to use for the invokation of gcloud.
+        /// The project id of the project to use for the invocation of gcloud.
         /// </summary>
         public string ProjectId { get; }
 
         protected readonly Dictionary<string, string> Environment = new Dictionary<string, string>
         {
             [GCloudMetricsVariable] = GoogleCloudExtensionPackage.Instance.ApplicationName,
-            [GCloudMetricsVersionVariable] =
-                GoogleCloudExtensionPackage.Instance.ApplicationVersion
+            [GCloudMetricsVersionVariable] = GoogleCloudExtensionPackage.Instance.ApplicationVersion
         };
 
+        private readonly Task<CloudSdkVersions> _versionsTask;
+
         /// <summary>
         /// Creates the default GCloud context from the current environment.
         /// </summary>
         public GCloudContext()
         {
             CredentialsPath = CredentialsStore.Default.CurrentAccountPath;
             ProjectId = CredentialsStore.Default.CurrentProjectId;
+            _versionsTask = GetGcloudOutputAsync<CloudSdkVersions>("version");
         }
 
         /// <summary>
@@ -92,10 +107,13 @@ public Task<bool> DeployAppAsync(string appYaml, string version, bool promote, A
         /// <param name="imageTag">The name of the image to build.</param>
         /// <param name="contentsPath">The contents of the container, including the Dockerfile.</param>
         /// <param name="outputAction">The action to perform on each line of output.</param>
-        public Task<bool> BuildContainerAsync(string imageTag, string contentsPath, Action<string> outputAction)
+        public async Task<bool> BuildContainerAsync(string imageTag, string contentsPath, Action<string> outputAction)
         {
-            string command = $"container builds submit --tag=\"{imageTag}\" \"{contentsPath}\"";
-            return RunGcloudCommandAsync(command, outputAction);
+            CloudSdkVersions sdkVersions = await _versionsTask;
+            string group = sdkVersions.SdkVersion >= s_gCloudBuildsMinimumVersion ? "builds" : "container builds";
+
+            string command = $"{group} submit --tag=\"{imageTag}\" \"{contentsPath}\"";
+            return await RunGcloudCommandAsync(command, outputAction);
         }
 
         /// <summary>

GoogleCloudExtension/GoogleCloudExtension/GCloud/GCloudWrapper.cs

@@ -45,7 +45,7 @@ public class GCloudWrapper : IGCloudWrapper
             new Dictionary<GCloudComponent, string>
             {
                 [GCloudComponent.Beta] = "beta",
-                [GCloudComponent.Kubectl] = "kubectl",
+                [GCloudComponent.Kubectl] = "kubectl"
             };
 
         private readonly Lazy<IProcessService> _processService;
@@ -89,7 +89,7 @@ public async Task<GCloudValidationResult> ValidateGCloudAsync(GCloudComponent co
         /// in <paramref name="outputPath"/>. If the <paramref name="sourcePath"/> does not refer to a supported CVS (currently git) then
         /// nothing will be done.
         /// </summary>
-        /// <param name="sourcePath">The directory for which to generate the source contenxt.</param>
+        /// <param name="sourcePath">The directory for which to generate the source context.</param>
         /// <param name="outputPath">Where to store the source context files.</param>
         /// <returns>The task to be completed when the operation finishes.</returns>
         public async Task GenerateSourceContextAsync(string sourcePath, string outputPath)
@@ -109,7 +109,7 @@ private async Task<IList<string>> GetInstalledComponentsAsync()
             return components.Where(x => x.State.IsInstalled).Select(x => x.Id).ToList();
         }
 
-        private bool IsGCloudCliInstalled()
+        private static bool IsGCloudCliInstalled()
         {
             Debug.WriteLine("Validating GCloud installation.");
             string gcloudPath = PathUtils.GetCommandPathFromPATH("gcloud.cmd");
@@ -136,7 +136,7 @@ private async Task<Version> GetInstalledCloudSdkVersionAsync()
             }
 
             CloudSdkVersions version = await GetJsonOutputAsync<CloudSdkVersions>("version");
-            return new Version(version.SdkVersion);
+            return version.SdkVersion;
         }
 
         private async Task<T> GetJsonOutputAsync<T>(string command)

GoogleCloudExtension/GoogleCloudExtension/GoogleCloudExtension.csproj

@@ -44,6 +44,7 @@
     <WarningLevel>4</WarningLevel>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
+    <LangVersion>7.1</LangVersion>
   </PropertyGroup>
   <PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
     <DebugType>pdbonly</DebugType>
@@ -54,6 +55,7 @@
     <WarningLevel>4</WarningLevel>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
+    <LangVersion>7.1</LangVersion>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="Accounts\CredentialsStore.cs" />

GoogleCloudExtension/GoogleCloudExtensionUnitTests/GCloud/GCloudContextUnitTests.cs

@@ -13,11 +13,14 @@
 // limitations under the License.
 
 using GoogleCloudExtension.GCloud;
+using GoogleCloudExtension.GCloud.Models;
 using GoogleCloudExtension.Utils;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
 using System;
 using System.Collections.Generic;
+using System.Linq;
+using System.Linq.Expressions;
 using System.Threading.Tasks;
 
 namespace GoogleCloudExtensionUnitTests.GCloud
@@ -36,11 +39,34 @@ public class GCloudContextUnitTests : ExtensionTestBase
         private GCloudContext _objectUnderTest;
         private Mock<IProcessService> _processServiceMock;
         private Action<string> _mockedOutputAction;
+        private TaskCompletionSource<CloudSdkVersions> _versionResultSource;
+
+        /// <summary>
+        /// A version of Google Cloud SDK that includes the gcloud builds commands.
+        /// </summary>
+        private static readonly CloudSdkVersions s_buildsEnabledSdkVersion =
+            new CloudSdkVersions { SdkVersion = new Version(GCloudContext.GCloudBuildsMinimumVersion) };
+
+        /// <summary>
+        /// A version of Google Cloud SDK from before the gcloud builds commands were added.
+        /// </summary>
+        private static readonly CloudSdkVersions s_buildsMissingSdkVersion =
+            new CloudSdkVersions { SdkVersion = new Version(GCloudWrapper.GCloudSdkMinimumVersion) };
+
+        /// <summary>
+        /// Used as dynamic data to test that container builder arguments do not change between versions.
+        /// </summary>
+        private static IEnumerable<object[]> SdkVersions => new[]
+        {
+            new object[] {s_buildsMissingSdkVersion},
+            new object[] {s_buildsEnabledSdkVersion}
+        };
 
         protected override void BeforeEach()
         {
             _processServiceMock = new Mock<IProcessService>();
-            SetupRunCommandResult(true);
+            _versionResultSource = new TaskCompletionSource<CloudSdkVersions>();
+            SetupGetJsonOutput("version", _versionResultSource.Task);
             PackageMock.Setup(p => p.ProcessService).Returns(_processServiceMock.Object);
             _objectUnderTest = new GCloudContext();
             _mockedOutputAction = Mock.Of<Action<string>>();
@@ -244,34 +270,51 @@ public async Task TestDeployAppAsync_ReturnsResultFromCommand(bool expectedResul
         }
 
         [TestMethod]
-        public async Task TestBuildContainerAsync_RunsGcloudContainerBuildsSubmit()
+        public async Task TestBuildContainerAsync_ForOldVersion_RunsGcloudContainerBuildsSubmit()
         {
+            _versionResultSource.SetResult(s_buildsMissingSdkVersion);
             await _objectUnderTest.BuildContainerAsync(DefaultImageTag, DefaultContentsPath, _mockedOutputAction);
 
-            VerifyCommandArgsContain("container builds submit");
+            VerifyCommandArgsContain("gcloud container builds submit");
         }
 
         [TestMethod]
-        public async Task TestBuildContainerAsync_PassesGivenImageTag()
+        public async Task TestBuildContainerAsync_ForNewerVersion_RunsGcloudBuildsSubmit()
         {
+            _versionResultSource.SetResult(s_buildsEnabledSdkVersion);
+            await _objectUnderTest.BuildContainerAsync(DefaultImageTag, DefaultContentsPath, _mockedOutputAction);
+
+            VerifyCommandArgsContain("gcloud builds submit");
+            VerifyCommandArgs(s => !s.Contains("container"));
+        }
+
+        [TestMethod]
+        [DynamicData(nameof(SdkVersions))]
+        public async Task TestBuildContainerAsync_PassesGivenImageTag(CloudSdkVersions version)
+        {
+            _versionResultSource.SetResult(version);
             const string expectedImageTag = "expected-image-tag";
             await _objectUnderTest.BuildContainerAsync(expectedImageTag, DefaultContentsPath, _mockedOutputAction);
 
             VerifyCommandArgsContain($"--tag=\"{expectedImageTag}\"");
         }
 
         [TestMethod]
-        public async Task TestBuildContainerAsync_PassesGivenIContentPath()
+        [DynamicData(nameof(SdkVersions))]
+        public async Task TestBuildContainerAsync_PassesGivenIContentPath(CloudSdkVersions version)
         {
+            _versionResultSource.SetResult(version);
             const string expectedContentsPath = "expected-contents-path";
             await _objectUnderTest.BuildContainerAsync(DefaultImageTag, expectedContentsPath, _mockedOutputAction);
 
             VerifyCommandArgsContain($"\"{expectedContentsPath}\"");
         }
 
         [TestMethod]
-        public async Task TestBuildContainerAsync_PassesHandler()
+        [DynamicData(nameof(SdkVersions))]
+        public async Task TestBuildContainerAsync_PassesHandler(CloudSdkVersions version)
         {
+            _versionResultSource.SetResult(version);
             const string expectedOutputLine = "expected-output-line";
             SetupRunCommandInvokeHandler(expectedOutputLine);
 
@@ -280,11 +323,16 @@ public async Task TestBuildContainerAsync_PassesHandler()
             Mock.Get(_mockedOutputAction).Verify(f => f(expectedOutputLine));
         }
 
+        private static IEnumerable<object[]> SdkVersionAndBooleans =>
+            SdkVersions.SelectMany(v => new[] { true, false }, (v, b) => new[] { v[0], b });
+
         [TestMethod]
-        [DataRow(true)]
-        [DataRow(false)]
-        public async Task TestBuildContainerAsync_ReturnsResultFromCommand(bool expectedResult)
+        [DynamicData(nameof(SdkVersionAndBooleans))]
+        public async Task TestBuildContainerAsync_ReturnsResultFromCommand(
+            CloudSdkVersions version,
+            bool expectedResult)
         {
+            _versionResultSource.SetResult(version);
             SetupRunCommandResult(expectedResult);
 
             bool result = await _objectUnderTest.BuildContainerAsync(
@@ -305,12 +353,14 @@ private void VerifyCommandOutputArgsContain<T>(string expectedArg)
                     It.IsAny<IDictionary<string, string>>()));
         }
 
-        private void VerifyCommandArgsContain(string expectedArg)
+        private void VerifyCommandArgsContain(string expectedArg) => VerifyCommandArgs(s => s.Contains(expectedArg));
+
+        private void VerifyCommandArgs(Expression<Func<string, bool>> predicateExpression)
         {
             _processServiceMock.Verify(
                 p => p.RunCommandAsync(
                     "cmd.exe",
-                    It.Is<string>(s => s.Contains(expectedArg)),
+                    It.Is(predicateExpression),
                     It.IsAny<EventHandler<OutputHandlerEventArgs>>(),
                     It.IsAny<string>(),
                     It.IsAny<IDictionary<string, string>>()));
@@ -329,6 +379,18 @@ private void SetupRunCommandResult(bool result)
                 .Returns(Task.FromResult(result));
         }
 
+        private void SetupGetJsonOutput<T>(string command, Task<T> result)
+        {
+            _processServiceMock
+                .Setup(
+                    p => p.GetJsonOutputAsync<T>(
+                        It.IsAny<string>(),
+                        It.Is<string>(s => s.Contains(command)),
+                        null,
+                        It.IsAny<Dictionary<string, string>>()))
+                .Returns(result);
+        }
+
         private void SetupGetJsonOutput<T>(T result)
         {
             _processServiceMock