Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added release logic to Optimization Engine #703

Open
wants to merge 58 commits into
base: features/aoe
Choose a base branch
from

Conversation

helderpinto
Copy link
Member

@helderpinto helderpinto commented May 14, 2024

πŸ› οΈ Description

Changed build, package and publish scripts to include the required assets for the Optimization Engine release and deployment process. The docs/deploy folder includes files that are just copies coming from the src/optimization-engine folder.

Also included minor bug fixes in workbooks and runbooks.

πŸ“‹ Checklist

πŸ”¬ How did you test this change?

  • 🀏 Lint tests
  • 🀞 PS -WhatIf / az validate
  • πŸ‘ Manually deployed + verified
  • πŸ’ͺ Unit tests
  • πŸ™Œ Integration tests

πŸ™‹β€β™€οΈ Do any of the following that apply?

  • 🚨 This is a breaking change.
  • 🀏 The change is less than 20 lines of code.

πŸ“‘ Did you update docs/changelog.md?

  • βœ… Yes (required for dev PRs)
  • ➑️ Will cover in a future PR (feature branch PRs only)
  • ❎ Not needed (small/internal change)

πŸ“– Did you update documentation?

  • βœ… Public docs in docs (required for dev)
  • βœ… Internal dev docs in src (required for dev)
  • ➑️ Will cover in a future PR (feature branch PRs only)
  • ❎ Not needed (small/internal change)

docs/deploy/finops-hub-0.2.1-rc.2.json Outdated Show resolved Hide resolved
docs/deploy/optimization-engine/azuredeploy-nested.bicep Outdated Show resolved Hide resolved
@@ -118,3 +118,28 @@ Get-ChildItem "$PSScriptRoot/../templates/$Template*" -Directory -ErrorAction Si

Write-Host ''
}

# Package optimization engine
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we move this into a separate Build-OptimizationEngine script? πŸ€” I'm fine keeping it here. Just thinking out loud. Fwiw, the templates should probably be moved to a Build-Template script as well. Alternatively, we can keep it here for now and define a more general approach to solve this consistently. I'm fine with anything. Just wanted to share that to see what your thoughts are.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, if we move templates and AOE to a different script, why don't we move workbooks and Bicep registry modules as well? Maybe each toolkit component type should have its own build script following some naming pattern and then we had the Build-Toolkit script calling all of them, provided the component directory (templates, workbooks, aoe, etc.) had a build script... But I'd keep it as is for now. I am adding a TODO to review the logic. If you prefer, I can open an issue for this.

Copy-Item "$path/azuredeploy.json" "$deployDir/$($path.Name)-latest.json"

$packageManifestPath = "$path/package-manifest.json"
if (Test-Path $packageManifestPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Would it be better to define a fallback manifest instead of just falling back to the previous logic? That way, we always work with a manifest, we just define a default one. I'm okay to not do that or maybe just add a TODO comment. Just a thought that crossed my mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree. I was about to include this change, but then I thought: where do we define the fallback manifest? Inline or as a file placed somewhere in the src structure? As I didn't want to make this decision now, I am leaving a TODO comment for now.

src/scripts/Publish-Toolkit.ps1 Show resolved Hide resolved
@flanakin flanakin added this to the May 2024 milestone May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Review πŸ‘€ PR that is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants