Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

refactor: vmss + application insights #360

Merged
merged 28 commits into from
Jan 5, 2024
Merged

Conversation

FoSix
Copy link
Contributor

@FoSix FoSix commented Nov 16, 2023

No description provided.

@FoSix
Copy link
Contributor Author

FoSix commented Nov 16, 2023

/help

ChatOPS built in help:

Currently supported commands include:

  • /sca - run all SCA tests via pre-commit
  • /validate - run terraform validate
  • /plan - plan the infrastructure (only examples)
  • /apply - deploy the infrastructure and destroy afterwards (only examples)
  • /idempotence - test idempotence: deploy, plan and destroy afterwards (only examples).

The 1st command does not take arguments, the remaining take two:

  • paths - a space delimitied list of module paths
  • tf_version - (optional, defaults to the latest available) a space delimited list of Terraform versions to test the infrastrucure against.

Examples:

# run idempotence tests on listed modules with Terraform versions: 1.2 (latest patch available), 1.4 (latest patch available), 1.5.4.
/idempotence paths="examples/common_vmseries examples/panorama_standalone" tf_version="1.2 1.4 1.5.4"
# run validation tests with the latest available Terraform version on listed modules.
/validate paths="modules/vmseries modules/vnet examples/dedicated_vmseries"

@FoSix
Copy link
Contributor Author

FoSix commented Nov 16, 2023

/idempotence paths="examples/common_vmseries_and_autoscale examples/dedicated_vmseries_and_autoscale" tf_version="1.3 1.4 1.5 1.6"

Testing job ID: 6891470932
Job result: FAILURE

@FoSix
Copy link
Contributor Author

FoSix commented Nov 16, 2023

/idempotence paths="examples/common_vmseries examples/common_vmseries_and_autoscale examples/dedicated_vmseries examples/dedicated_vmseries_and_autoscale examples/gwlb_with_vmseries examples/standalone_panorama examples/standalone_vmseries" tf_version="1.5 1.6"

Testing job ID: 6892413937
Job result: FAILURE

@FoSix FoSix marked this pull request as ready for review November 17, 2023 08:17
@FoSix FoSix requested a review from a team as a code owner November 17, 2023 08:17
Copy link
Contributor

@acelebanski acelebanski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, let's merge if tested with all relevant examples.

@FoSix
Copy link
Contributor Author

FoSix commented Dec 8, 2023

/idempotence paths="examples/common_vmseries_and_autoscale examples/dedicated_vmseries_and_autoscale" tf_version="1.5 1.6"

Testing job ID: 7140698442
Job result: SUCCESS

@FoSix FoSix force-pushed the 335-vmss-refactor branch from daf4c2d to 18d1458 Compare December 8, 2023 11:48
@FoSix FoSix force-pushed the 335-vmss-refactor branch from 18d1458 to b24a7d1 Compare December 8, 2023 14:19
@FoSix FoSix marked this pull request as draft December 8, 2023 14:21
@FoSix FoSix force-pushed the 335-vmss-refactor branch from b24a7d1 to e32a009 Compare December 8, 2023 14:48
@FoSix
Copy link
Contributor Author

FoSix commented Dec 21, 2023

/idempotence paths="examples/common_vmseries examples/common_vmseries_and_autoscale examples/dedicated_vmseries examples/dedicated_vmseries_and_autoscale examples/standalone_panorama examples/standalone_vmseries" tf_version="1.5 1.6"

Testing job ID: 7286475574
Job result: FAILURE
Job result: FAILURE
Job result: FAILURE
Job result: SUCCESS

@FoSix FoSix force-pushed the 335-vmss-refactor branch from 523c051 to 2ee4e1f Compare December 21, 2023 15:57
@FoSix FoSix marked this pull request as ready for review December 21, 2023 16:40
@FoSix
Copy link
Contributor Author

FoSix commented Dec 22, 2023

/idempotence paths="examples/common_vmseries examples/common_vmseries_and_autoscale examples/dedicated_vmseries examples/dedicated_vmseries_and_autoscale examples/standalone_panorama examples/standalone_vmseries" tf_version="1.5 1.6"

Testing job ID: 7298785291
Job result: SUCCESS

Copy link
Contributor

@sebastianczech sebastianczech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job 👍

modules/ngfw_metrics/main_test.go Outdated Show resolved Hide resolved
modules/ngfw_metrics/versions.tf Outdated Show resolved Hide resolved

admin_username = var.authentication.username
admin_password = var.authentication.disable_password_authentication ? null : local.password
disable_password_authentication = var.authentication.disable_password_authentication
Copy link
Contributor

@sebastianczech sebastianczech Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about getting rid of var.authentication.disable_password_authentication and use:

 disable_password_authentication = length(var.authentication.ssh_keys) > 0 && var.authentication.password == null

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea for this was to allow having both SSH key & password for authentication.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

modules/vmss/main.tf Outdated Show resolved Hide resolved
modules/vmss/main.tf Outdated Show resolved Hide resolved
modules/vmss/main.tf Outdated Show resolved Hide resolved
end_time = string
}))
scale_rules = optional(list(object({
name = string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding validation if name is from 10 possible metrics:

DataPlaneCPUUtilizationPct
DataPlanePacketBufferUtilization
panGPGatewayUtilizationPct
panGPGWUtilizationActiveTunnels
panSessionActive
panSessionConnectionsPerSecond
panSessionSslProxyUtilization
panSessionThroughputKbps
panSessionThroughputPps
panSessionUtilization

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but name can be also a name of the built-in metric, we would have to maintain that list and follow changes on Azure. I'm not a fan of that :)

@FoSix FoSix merged commit 7e8b486 into 307-refactor-modules Jan 5, 2024
@FoSix FoSix deleted the 335-vmss-refactor branch January 5, 2024 10:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants