You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@fluo.apache.org by GitBox <gi...@apache.org> on 2021/05/20 20:00:17 UTC

[GitHub] [fluo-muchos] karthick-rn opened a new pull request #395: Add validation checks for Azure deployments

karthick-rn opened a new pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395


   This PR includes only additional checks for Azure deployments and does not change any functional behaviour.
   Addresses part3 in #392 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647815676



##########
File path: ansible/group_vars/.gitignore
##########
@@ -1 +1 @@
-/all
+/*

Review comment:
       The objective is to commit the `.gitignore` file & ignore the rest of the files in that directory. Also, that is the only dotfile in the directory. In general, I think git doesn't ignore the `.gitignore` files. For instance, if we add `.*` or `.gitignore` git still doesn't ignore it and `git status` reports the file has been modified. Not sure if it's a git thing. Just to be safe, I'll go ahead and explicitly whitelist as mentioned in the above link.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r648197985



##########
File path: lib/muchos/config/azurevalidations.py
##########
@@ -0,0 +1,253 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+#
+
+from .base import ConfigValidator
+from .azurevalidationhelpers import (
+    vmss_status_succeeded_if_exists,
+    vmss_cluster_has_appropriate_data_disk_count,
+    vmss_exists,
+)
+from azure.mgmt.compute import ComputeManagementClient
+from azure.common.client_factory import get_client_from_cli_profile
+
+
+def validate_azure_configs(config, action):
+    # get VM SKU resources for this location. we have to use
+    # a specific API version to do this as this resource_skus
+    # list operation is not allowed in any other API versions
+    # which are available with the version of Azure SDK
+    # that ships with Ansible for Azure
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2017-09-01"
+    )
+    config.vm_skus_for_location = list(
+        filter(
+            lambda s: s.resource_type == "virtualMachines"
+            and config.location() in s.locations,
+            config.client.resource_skus.list(),
+        )
+    )
+
+    # switch to 2018-06-01 API which has support for other operations
+    # including VMSS checks
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2018-06-01"
+    )
+
+    validations = (
+        AZURE_VALIDATIONS["common"] + AZURE_VALIDATIONS[action]
+        if action in AZURE_VALIDATIONS
+        else []
+    )
+    return list(
+        filter(
+            lambda r: isinstance(r, str),
+            map(lambda v: v(config, config.client), validations),
+        )
+    )
+
+
+AZURE_VALIDATIONS = {
+    "common": [
+        # if VMSS instances are pending upgrade to latest version
+        # block the execution of the setup phase.
+        ConfigValidator(
+            vmss_status_succeeded_if_exists,
+            "VMSS must not exist or be in 'Succeeded' state",
+        ),
+        # Validate that the data disk configuration is appropriate
+        # considering temp disk usage etc.
+        ConfigValidator(vmss_cluster_has_appropriate_data_disk_count, None),
+
+        ConfigValidator(lambda config, client: not config.use_multiple_vmss()),
+        # the VM SKU specified is not a valid Azure VM SKU
+        ConfigValidator(
+            lambda config, client: config.vm_sku()
+            in {s.name: s for s in config.vm_skus_for_location},
+            "azure.vm_sku must be a valid VM SKU for the selected location",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku")
+                    in {s.name: s for s in config.vm_skus_for_location}
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with sku "
+            "must be a valid VM SKU for the selected location",
+        ),
+        # managed_disk_type in
+        # ['Standard_LRS', 'StandardSSD_LRS', Premium_LRS']
+        ConfigValidator(
+            lambda config, client: config.managed_disk_type()
+            in ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"],
+            "managed_disk_type must be "
+            "one of Standard_LRS, StandardSSD_LRS, or Premium_LRS",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("disk_sku")
+                    in ["Standard_LRS",  "StandardSSD_LRS", "Premium_LRS"]

Review comment:
        I forgot about running `black`, thanks for pointing it. The reformatted code has been pushed via [e31ddc2](https://github.com/apache/fluo-muchos/pull/395/commits/e31ddc2e7d68683e05b4f0f87252b142375ce6d8) 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] keith-turner commented on pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
keith-turner commented on pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#issuecomment-846021439


   @karthick-rn  I am attempting to review this, but do not know python well enough to give a good review quickly. I am still looking at it though.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#issuecomment-856036592


   @arvindshmicrosoft Hi, Thanks for taking the time to review this PR and also for helping me out to refine this work further and cover few more checks. Based on our last discussion I have applied the changes in this [1c5c08f](https://github.com/apache/fluo-muchos/pull/395/commits/1c5c08f3d1f2367e094f8e9ec9ea02c759658408). 
   I have tested these changes against the following scenarios successfully
   - Standard Lsv2 NVMe with no data disks
   - Multiple VMSS with HA & without HA
   - ADLS Gen2 
   - With & without Azure proxy host
   
   I have also included a _TODO_ in `azure/test_config.py` for future work on adding test cases. 
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r646751111



##########
File path: ansible/group_vars/.gitignore
##########
@@ -1 +1 @@
-/all
+/*

Review comment:
       Is this trying to ignore everything in this directory in order to maintain an empty directory? If so, some tips to do that without ignoring the `.gitignore` file itself are at https://stackoverflow.com/a/932982/196405




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#issuecomment-846095143


   @arvindshmicrosoft ran into some issues with these changes, I'm currently looking into this and will update the PR.  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647300859



##########
File path: ansible/group_vars/.gitignore
##########
@@ -1 +1 @@
-/all
+/*

Review comment:
       Looks like the `.gitignore` file is not ignored and still being tracked even if I don't explicitly whitelist `!.gitignore` as specified in the link. I have tested the below scenarios, let me know if I'm missing anything? Thanks
   
   ```
   (env) [knarendran@kn-centos75-jbox fluo-muchos]$ git diff
   diff --git a/ansible/group_vars/.gitignore b/ansible/group_vars/.gitignore
   index 33662f5..4ef7868 100644
   --- a/ansible/group_vars/.gitignore
   +++ b/ansible/group_vars/.gitignore
   @@ -1 +1,2 @@
   -/*
   +# Ignore everything in this directory
   +*
   
   (env) [knarendran@kn-centos75-jbox fluo-muchos]$ git status
   # On branch karthick/azure_validations
   # Changes not staged for commit:
   #   (use "git add <file>..." to update what will be committed)
   #   (use "git checkout -- <file>..." to discard changes in working directory)
   #
   #       modified:   ansible/group_vars/.gitignore
   #
   no changes added to commit (use "git add" and/or "git commit -a")
   ```
   
   ```
   (env) [knarendran@kn-centos75-jbox fluo-muchos]$ git diff
   diff --git a/ansible/group_vars/.gitignore b/ansible/group_vars/.gitignore
   index 33662f5..5e7d273 100644
   --- a/ansible/group_vars/.gitignore
   +++ b/ansible/group_vars/.gitignore
   @@ -1 +1,4 @@
   -/*
   +# Ignore everything in this directory
   +*
   +# Except this file
   +!.gitignore
   
   (env) [knarendran@kn-centos75-jbox fluo-muchos]$ git status
   # On branch karthick/azure_validations
   # Changes not staged for commit:
   #   (use "git add <file>..." to update what will be committed)
   #   (use "git checkout -- <file>..." to discard changes in working directory)
   #
   #       modified:   ansible/group_vars/.gitignore
   #
   no changes added to commit (use "git add" and/or "git commit -a")
   ```
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] keith-turner commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647593734



##########
File path: ansible/roles/azure/tasks/create_optional_proxy.yml
##########
@@ -66,7 +66,7 @@
     name: "{{ azure_proxy_host }}"
     network_interface_names:
       - "{{ azure_proxy_host }}-nic"
-    vm_size: "{{ azure_proxy_host_vm_sku }}"
+    vm_size: Standard_D8s_v3

Review comment:
       Was removing the variable and replacing it with a literal intentional?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] arvindshmicrosoft commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r636567366



##########
File path: ansible/roles/azure/tasks/create_multiple_vmss.yml
##########
@@ -30,6 +30,23 @@
     file: "{{ deploy_path }}/conf/azure_multiple_vmss_vars.yml"
     name: azure_multiple_vmss_vars
 
+# Minimum check to ensure azure_multiple_vmss_vars.yml contains the right set of services
+- name: Fail when HA is not enabled and azure_multiple_vmss_vars.yml contains HA services
+  fail:
+    msg:  |
+          Incorrect roles in azure_multiple_vmss_vars.yml, 'journalnode' or 'zkfc' required only when HA is enabled.
+          Refer the VMSS definition in the example yaml file.
+  with_items: "{{ azure_multiple_vmss_vars.vars_list }}"
+  when: "('journalnode' in item.roles or 'zkfc' in item.roles) and not hdfs_ha"
+
+- name: Fail when HA is enabled and azure_multiple_vmss_vars.yml does not contain HA services

Review comment:
       I'm sorry if I missed something, but why introduce these validations here separately from the rest of the validations in the azurevalidations.py file?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn merged pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn merged pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] arvindshmicrosoft commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
arvindshmicrosoft commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647863400



##########
File path: lib/muchos/config/azurevalidations.py
##########
@@ -0,0 +1,253 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+#
+
+from .base import ConfigValidator
+from .azurevalidationhelpers import (
+    vmss_status_succeeded_if_exists,
+    vmss_cluster_has_appropriate_data_disk_count,
+    vmss_exists,
+)
+from azure.mgmt.compute import ComputeManagementClient
+from azure.common.client_factory import get_client_from_cli_profile
+
+
+def validate_azure_configs(config, action):
+    # get VM SKU resources for this location. we have to use
+    # a specific API version to do this as this resource_skus
+    # list operation is not allowed in any other API versions
+    # which are available with the version of Azure SDK
+    # that ships with Ansible for Azure
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2017-09-01"
+    )
+    config.vm_skus_for_location = list(
+        filter(
+            lambda s: s.resource_type == "virtualMachines"
+            and config.location() in s.locations,
+            config.client.resource_skus.list(),
+        )
+    )
+
+    # switch to 2018-06-01 API which has support for other operations
+    # including VMSS checks
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2018-06-01"
+    )
+
+    validations = (
+        AZURE_VALIDATIONS["common"] + AZURE_VALIDATIONS[action]
+        if action in AZURE_VALIDATIONS
+        else []
+    )
+    return list(
+        filter(
+            lambda r: isinstance(r, str),
+            map(lambda v: v(config, config.client), validations),
+        )
+    )
+
+
+AZURE_VALIDATIONS = {
+    "common": [
+        # if VMSS instances are pending upgrade to latest version
+        # block the execution of the setup phase.
+        ConfigValidator(
+            vmss_status_succeeded_if_exists,
+            "VMSS must not exist or be in 'Succeeded' state",
+        ),
+        # Validate that the data disk configuration is appropriate
+        # considering temp disk usage etc.
+        ConfigValidator(vmss_cluster_has_appropriate_data_disk_count, None),
+
+        ConfigValidator(lambda config, client: not config.use_multiple_vmss()),
+        # the VM SKU specified is not a valid Azure VM SKU
+        ConfigValidator(
+            lambda config, client: config.vm_sku()
+            in {s.name: s for s in config.vm_skus_for_location},
+            "azure.vm_sku must be a valid VM SKU for the selected location",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku")
+                    in {s.name: s for s in config.vm_skus_for_location}
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with sku "
+            "must be a valid VM SKU for the selected location",
+        ),
+        # managed_disk_type in
+        # ['Standard_LRS', 'StandardSSD_LRS', Premium_LRS']
+        ConfigValidator(
+            lambda config, client: config.managed_disk_type()
+            in ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"],
+            "managed_disk_type must be "
+            "one of Standard_LRS, StandardSSD_LRS, or Premium_LRS",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("disk_sku")
+                    in ["Standard_LRS",  "StandardSSD_LRS", "Premium_LRS"]
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with disk_sku must "
+            "be one of Standard_LRS, StandardSSD_LRS or Premium_LRS",
+        ),
+        # Cannot specify Premium managed disks if VMSS SKU is / are not capable
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or not config.managed_disk_type() == "Premium_LRS"
+            or config.vm_sku() in config.premiumio_capable_skus(),
+            "azure.vm_sku must be Premium I/O capable VM SKU "
+            "in order to use Premium Managed Disks",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku") in config.premiumio_capable_skus()
+                    if vmss.get("disk_sku") == "Premium_LRS"
+                    else True
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS set to use Premium "
+            "Managed Disks must use a Premium I/O capable VM SKU",
+        ),
+        # Data disk count specified cannot exceed MaxDataDisks for VM SKU
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or config.data_disk_count()
+            <= config.max_data_disks_for_skus()[config.vm_sku()],

Review comment:
       ```suggestion
               <= config.max_data_disks_for_skus().get(config.vm_sku(), 0),
   ```

##########
File path: lib/muchos/config/azurevalidations.py
##########
@@ -0,0 +1,253 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+#
+
+from .base import ConfigValidator
+from .azurevalidationhelpers import (
+    vmss_status_succeeded_if_exists,
+    vmss_cluster_has_appropriate_data_disk_count,
+    vmss_exists,
+)
+from azure.mgmt.compute import ComputeManagementClient
+from azure.common.client_factory import get_client_from_cli_profile
+
+
+def validate_azure_configs(config, action):
+    # get VM SKU resources for this location. we have to use
+    # a specific API version to do this as this resource_skus
+    # list operation is not allowed in any other API versions
+    # which are available with the version of Azure SDK
+    # that ships with Ansible for Azure
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2017-09-01"
+    )
+    config.vm_skus_for_location = list(
+        filter(
+            lambda s: s.resource_type == "virtualMachines"
+            and config.location() in s.locations,
+            config.client.resource_skus.list(),
+        )
+    )
+
+    # switch to 2018-06-01 API which has support for other operations
+    # including VMSS checks
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2018-06-01"
+    )
+
+    validations = (
+        AZURE_VALIDATIONS["common"] + AZURE_VALIDATIONS[action]
+        if action in AZURE_VALIDATIONS
+        else []
+    )
+    return list(
+        filter(
+            lambda r: isinstance(r, str),
+            map(lambda v: v(config, config.client), validations),
+        )
+    )
+
+
+AZURE_VALIDATIONS = {
+    "common": [
+        # if VMSS instances are pending upgrade to latest version
+        # block the execution of the setup phase.
+        ConfigValidator(
+            vmss_status_succeeded_if_exists,
+            "VMSS must not exist or be in 'Succeeded' state",
+        ),
+        # Validate that the data disk configuration is appropriate
+        # considering temp disk usage etc.
+        ConfigValidator(vmss_cluster_has_appropriate_data_disk_count, None),
+
+        ConfigValidator(lambda config, client: not config.use_multiple_vmss()),
+        # the VM SKU specified is not a valid Azure VM SKU
+        ConfigValidator(
+            lambda config, client: config.vm_sku()
+            in {s.name: s for s in config.vm_skus_for_location},
+            "azure.vm_sku must be a valid VM SKU for the selected location",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku")
+                    in {s.name: s for s in config.vm_skus_for_location}
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with sku "
+            "must be a valid VM SKU for the selected location",
+        ),
+        # managed_disk_type in
+        # ['Standard_LRS', 'StandardSSD_LRS', Premium_LRS']
+        ConfigValidator(
+            lambda config, client: config.managed_disk_type()
+            in ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"],
+            "managed_disk_type must be "
+            "one of Standard_LRS, StandardSSD_LRS, or Premium_LRS",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("disk_sku")
+                    in ["Standard_LRS",  "StandardSSD_LRS", "Premium_LRS"]
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with disk_sku must "
+            "be one of Standard_LRS, StandardSSD_LRS or Premium_LRS",
+        ),
+        # Cannot specify Premium managed disks if VMSS SKU is / are not capable
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or not config.managed_disk_type() == "Premium_LRS"
+            or config.vm_sku() in config.premiumio_capable_skus(),
+            "azure.vm_sku must be Premium I/O capable VM SKU "
+            "in order to use Premium Managed Disks",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku") in config.premiumio_capable_skus()
+                    if vmss.get("disk_sku") == "Premium_LRS"
+                    else True
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS set to use Premium "
+            "Managed Disks must use a Premium I/O capable VM SKU",
+        ),
+        # Data disk count specified cannot exceed MaxDataDisks for VM SKU
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or config.data_disk_count()
+            <= config.max_data_disks_for_skus()[config.vm_sku()],
+            "Number of data disks specified exceeds allowed limit for VM SKU",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("data_disk_count")
+                    <= config.max_data_disks_for_skus()[vmss.get("sku")]

Review comment:
       ```suggestion
                       <= config.max_data_disks_for_skus().get(config.vm_sku(), 0)
   ```

##########
File path: lib/muchos/config/azurevalidations.py
##########
@@ -0,0 +1,253 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+#
+
+from .base import ConfigValidator
+from .azurevalidationhelpers import (
+    vmss_status_succeeded_if_exists,
+    vmss_cluster_has_appropriate_data_disk_count,
+    vmss_exists,
+)
+from azure.mgmt.compute import ComputeManagementClient
+from azure.common.client_factory import get_client_from_cli_profile
+
+
+def validate_azure_configs(config, action):
+    # get VM SKU resources for this location. we have to use
+    # a specific API version to do this as this resource_skus
+    # list operation is not allowed in any other API versions
+    # which are available with the version of Azure SDK
+    # that ships with Ansible for Azure
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2017-09-01"
+    )
+    config.vm_skus_for_location = list(
+        filter(
+            lambda s: s.resource_type == "virtualMachines"
+            and config.location() in s.locations,
+            config.client.resource_skus.list(),
+        )
+    )
+
+    # switch to 2018-06-01 API which has support for other operations
+    # including VMSS checks
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2018-06-01"
+    )
+
+    validations = (
+        AZURE_VALIDATIONS["common"] + AZURE_VALIDATIONS[action]
+        if action in AZURE_VALIDATIONS
+        else []
+    )
+    return list(
+        filter(
+            lambda r: isinstance(r, str),
+            map(lambda v: v(config, config.client), validations),
+        )
+    )
+
+
+AZURE_VALIDATIONS = {
+    "common": [
+        # if VMSS instances are pending upgrade to latest version
+        # block the execution of the setup phase.
+        ConfigValidator(
+            vmss_status_succeeded_if_exists,
+            "VMSS must not exist or be in 'Succeeded' state",
+        ),
+        # Validate that the data disk configuration is appropriate
+        # considering temp disk usage etc.
+        ConfigValidator(vmss_cluster_has_appropriate_data_disk_count, None),
+
+        ConfigValidator(lambda config, client: not config.use_multiple_vmss()),
+        # the VM SKU specified is not a valid Azure VM SKU
+        ConfigValidator(
+            lambda config, client: config.vm_sku()
+            in {s.name: s for s in config.vm_skus_for_location},
+            "azure.vm_sku must be a valid VM SKU for the selected location",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku")
+                    in {s.name: s for s in config.vm_skus_for_location}
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with sku "
+            "must be a valid VM SKU for the selected location",
+        ),
+        # managed_disk_type in
+        # ['Standard_LRS', 'StandardSSD_LRS', Premium_LRS']
+        ConfigValidator(
+            lambda config, client: config.managed_disk_type()
+            in ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"],
+            "managed_disk_type must be "
+            "one of Standard_LRS, StandardSSD_LRS, or Premium_LRS",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("disk_sku")
+                    in ["Standard_LRS",  "StandardSSD_LRS", "Premium_LRS"]

Review comment:
       Nit: extra space. Please do run `black lib --line-length 79` as recommended in the [CONTRIBUTING](https://github.com/apache/fluo-muchos/blob/main/CONTRIBUTING.md#before-you-submit-a-pr) guidelines.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647575845



##########
File path: ansible/group_vars/.gitignore
##########
@@ -1 +1 @@
-/all
+/*

Review comment:
       It looks like it works without excluding it because `*` doesn't match on dotfiles (so no dotfiles would be ignored). However, that might be platform-dependent, since dotfiles are considered "hidden" on \*nix systems, but not necessarily on others. The file-globbing pattern matching might be different on different systems... I'm not sure. If you wanted to ignore dotfiles, too, then you would probably have to add `.*` and `!.gitignore` also. I'm fine with whatever works, though.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#issuecomment-847702495


   I'm revisiting some parts of the code in this PR. Will update the PR once the testing is complete. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] ctubbsii commented on pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#issuecomment-857957337


   > I have no more comments on this, and am approving. But please do follow through on @ctubbsii observations.
   
   I don't have anything further. Feel free to merge if you're happy with the other reviewers


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647575845



##########
File path: ansible/group_vars/.gitignore
##########
@@ -1 +1 @@
-/all
+/*

Review comment:
       It looks like it works without excluding it because `*` doesn't match on dotfiles (so no dotfiles would be ignored). However, that might be platform-dependent, since dotfiles are considered "hidden" on \*nix systems, but not necessarily on others. The file-globbing pattern matching might be different on different systems... I'm not sure. If you wanted to ignore dotfiles, too, then you would probably have to add `.*` and `!.gitignore` explicitly. I'm fine with whatever works, though.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#issuecomment-857959613


   Thanks for all your comments and time in reviewing this PR. I'll go ahead and merge it. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r648195995



##########
File path: lib/muchos/config/azurevalidations.py
##########
@@ -0,0 +1,253 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+#
+
+from .base import ConfigValidator
+from .azurevalidationhelpers import (
+    vmss_status_succeeded_if_exists,
+    vmss_cluster_has_appropriate_data_disk_count,
+    vmss_exists,
+)
+from azure.mgmt.compute import ComputeManagementClient
+from azure.common.client_factory import get_client_from_cli_profile
+
+
+def validate_azure_configs(config, action):
+    # get VM SKU resources for this location. we have to use
+    # a specific API version to do this as this resource_skus
+    # list operation is not allowed in any other API versions
+    # which are available with the version of Azure SDK
+    # that ships with Ansible for Azure
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2017-09-01"
+    )
+    config.vm_skus_for_location = list(
+        filter(
+            lambda s: s.resource_type == "virtualMachines"
+            and config.location() in s.locations,
+            config.client.resource_skus.list(),
+        )
+    )
+
+    # switch to 2018-06-01 API which has support for other operations
+    # including VMSS checks
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2018-06-01"
+    )
+
+    validations = (
+        AZURE_VALIDATIONS["common"] + AZURE_VALIDATIONS[action]
+        if action in AZURE_VALIDATIONS
+        else []
+    )
+    return list(
+        filter(
+            lambda r: isinstance(r, str),
+            map(lambda v: v(config, config.client), validations),
+        )
+    )
+
+
+AZURE_VALIDATIONS = {
+    "common": [
+        # if VMSS instances are pending upgrade to latest version
+        # block the execution of the setup phase.
+        ConfigValidator(
+            vmss_status_succeeded_if_exists,
+            "VMSS must not exist or be in 'Succeeded' state",
+        ),
+        # Validate that the data disk configuration is appropriate
+        # considering temp disk usage etc.
+        ConfigValidator(vmss_cluster_has_appropriate_data_disk_count, None),
+
+        ConfigValidator(lambda config, client: not config.use_multiple_vmss()),
+        # the VM SKU specified is not a valid Azure VM SKU
+        ConfigValidator(
+            lambda config, client: config.vm_sku()
+            in {s.name: s for s in config.vm_skus_for_location},
+            "azure.vm_sku must be a valid VM SKU for the selected location",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku")
+                    in {s.name: s for s in config.vm_skus_for_location}
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with sku "
+            "must be a valid VM SKU for the selected location",
+        ),
+        # managed_disk_type in
+        # ['Standard_LRS', 'StandardSSD_LRS', Premium_LRS']
+        ConfigValidator(
+            lambda config, client: config.managed_disk_type()
+            in ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"],
+            "managed_disk_type must be "
+            "one of Standard_LRS, StandardSSD_LRS, or Premium_LRS",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("disk_sku")
+                    in ["Standard_LRS",  "StandardSSD_LRS", "Premium_LRS"]
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with disk_sku must "
+            "be one of Standard_LRS, StandardSSD_LRS or Premium_LRS",
+        ),
+        # Cannot specify Premium managed disks if VMSS SKU is / are not capable
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or not config.managed_disk_type() == "Premium_LRS"
+            or config.vm_sku() in config.premiumio_capable_skus(),
+            "azure.vm_sku must be Premium I/O capable VM SKU "
+            "in order to use Premium Managed Disks",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku") in config.premiumio_capable_skus()
+                    if vmss.get("disk_sku") == "Premium_LRS"
+                    else True
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS set to use Premium "
+            "Managed Disks must use a Premium I/O capable VM SKU",
+        ),
+        # Data disk count specified cannot exceed MaxDataDisks for VM SKU
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or config.data_disk_count()
+            <= config.max_data_disks_for_skus()[config.vm_sku()],

Review comment:
       done via [e31ddc2](https://github.com/apache/fluo-muchos/pull/395/commits/e31ddc2e7d68683e05b4f0f87252b142375ce6d8) 

##########
File path: lib/muchos/config/azurevalidations.py
##########
@@ -0,0 +1,253 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+#
+
+from .base import ConfigValidator
+from .azurevalidationhelpers import (
+    vmss_status_succeeded_if_exists,
+    vmss_cluster_has_appropriate_data_disk_count,
+    vmss_exists,
+)
+from azure.mgmt.compute import ComputeManagementClient
+from azure.common.client_factory import get_client_from_cli_profile
+
+
+def validate_azure_configs(config, action):
+    # get VM SKU resources for this location. we have to use
+    # a specific API version to do this as this resource_skus
+    # list operation is not allowed in any other API versions
+    # which are available with the version of Azure SDK
+    # that ships with Ansible for Azure
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2017-09-01"
+    )
+    config.vm_skus_for_location = list(
+        filter(
+            lambda s: s.resource_type == "virtualMachines"
+            and config.location() in s.locations,
+            config.client.resource_skus.list(),
+        )
+    )
+
+    # switch to 2018-06-01 API which has support for other operations
+    # including VMSS checks
+    config.client = get_client_from_cli_profile(
+        ComputeManagementClient, api_version="2018-06-01"
+    )
+
+    validations = (
+        AZURE_VALIDATIONS["common"] + AZURE_VALIDATIONS[action]
+        if action in AZURE_VALIDATIONS
+        else []
+    )
+    return list(
+        filter(
+            lambda r: isinstance(r, str),
+            map(lambda v: v(config, config.client), validations),
+        )
+    )
+
+
+AZURE_VALIDATIONS = {
+    "common": [
+        # if VMSS instances are pending upgrade to latest version
+        # block the execution of the setup phase.
+        ConfigValidator(
+            vmss_status_succeeded_if_exists,
+            "VMSS must not exist or be in 'Succeeded' state",
+        ),
+        # Validate that the data disk configuration is appropriate
+        # considering temp disk usage etc.
+        ConfigValidator(vmss_cluster_has_appropriate_data_disk_count, None),
+
+        ConfigValidator(lambda config, client: not config.use_multiple_vmss()),
+        # the VM SKU specified is not a valid Azure VM SKU
+        ConfigValidator(
+            lambda config, client: config.vm_sku()
+            in {s.name: s for s in config.vm_skus_for_location},
+            "azure.vm_sku must be a valid VM SKU for the selected location",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku")
+                    in {s.name: s for s in config.vm_skus_for_location}
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with sku "
+            "must be a valid VM SKU for the selected location",
+        ),
+        # managed_disk_type in
+        # ['Standard_LRS', 'StandardSSD_LRS', Premium_LRS']
+        ConfigValidator(
+            lambda config, client: config.managed_disk_type()
+            in ["Standard_LRS", "StandardSSD_LRS", "Premium_LRS"],
+            "managed_disk_type must be "
+            "one of Standard_LRS, StandardSSD_LRS, or Premium_LRS",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("disk_sku")
+                    in ["Standard_LRS",  "StandardSSD_LRS", "Premium_LRS"]
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS with disk_sku must "
+            "be one of Standard_LRS, StandardSSD_LRS or Premium_LRS",
+        ),
+        # Cannot specify Premium managed disks if VMSS SKU is / are not capable
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or not config.managed_disk_type() == "Premium_LRS"
+            or config.vm_sku() in config.premiumio_capable_skus(),
+            "azure.vm_sku must be Premium I/O capable VM SKU "
+            "in order to use Premium Managed Disks",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("sku") in config.premiumio_capable_skus()
+                    if vmss.get("disk_sku") == "Premium_LRS"
+                    else True
+                    for vmss in config.azure_multiple_vmss_vars.get(
+                        "vars_list", []
+                    )
+                ]
+            ),
+            "when use_multiple_vmss == True, any VMSS set to use Premium "
+            "Managed Disks must use a Premium I/O capable VM SKU",
+        ),
+        # Data disk count specified cannot exceed MaxDataDisks for VM SKU
+        ConfigValidator(
+            lambda config, client: config.use_multiple_vmss()
+            or config.data_disk_count()
+            <= config.max_data_disks_for_skus()[config.vm_sku()],
+            "Number of data disks specified exceeds allowed limit for VM SKU",
+        ),
+        ConfigValidator(
+            lambda config, client: not config.use_multiple_vmss()
+            or all(
+                [
+                    vmss.get("data_disk_count")
+                    <= config.max_data_disks_for_skus()[vmss.get("sku")]

Review comment:
       done via [e31ddc2](https://github.com/apache/fluo-muchos/pull/395/commits/e31ddc2e7d68683e05b4f0f87252b142375ce6d8) 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r646703940



##########
File path: ansible/roles/azure/tasks/create_multiple_vmss.yml
##########
@@ -30,6 +30,23 @@
     file: "{{ deploy_path }}/conf/azure_multiple_vmss_vars.yml"
     name: azure_multiple_vmss_vars
 
+# Minimum check to ensure azure_multiple_vmss_vars.yml contains the right set of services
+- name: Fail when HA is not enabled and azure_multiple_vmss_vars.yml contains HA services
+  fail:
+    msg:  |
+          Incorrect roles in azure_multiple_vmss_vars.yml, 'journalnode' or 'zkfc' required only when HA is enabled.
+          Refer the VMSS definition in the example yaml file.
+  with_items: "{{ azure_multiple_vmss_vars.vars_list }}"
+  when: "('journalnode' in item.roles or 'zkfc' in item.roles) and not hdfs_ha"
+
+- name: Fail when HA is enabled and azure_multiple_vmss_vars.yml does not contain HA services

Review comment:
       Addressed this in [1c5c08f](https://github.com/apache/fluo-muchos/pull/395/commits/1c5c08f3d1f2367e094f8e9ec9ea02c759658408)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] ctubbsii commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647575845



##########
File path: ansible/group_vars/.gitignore
##########
@@ -1 +1 @@
-/all
+/*

Review comment:
       It looks like it works without excluding it because `*` doesn't match on dotfiles. However, that might be platform-dependent, since dotfiles are considered "hidden" on \*nix systems, but not necessarily on others. I'm not sure how git works across operating systems. If you wanted to ignore dotfiles, too, then you would probably have to add `.*` and `!.gitignore` also. I'm fine with whatever works, though.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [fluo-muchos] karthick-rn commented on a change in pull request #395: Add validation checks for Azure deployments

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #395:
URL: https://github.com/apache/fluo-muchos/pull/395#discussion_r647643512



##########
File path: ansible/roles/azure/tasks/create_optional_proxy.yml
##########
@@ -66,7 +66,7 @@
     name: "{{ azure_proxy_host }}"
     network_interface_names:
       - "{{ azure_proxy_host }}-nic"
-    vm_size: "{{ azure_proxy_host_vm_sku }}"
+    vm_size: Standard_D8s_v3

Review comment:
       Yes (in the interim). After this work, I'll add this variable to the `muchos.props` file and make it more generic in a separate PR. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org