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.

Sign up for GitHub

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

Jump to bottom

Pipeline updates #1072

Merged
merged 27 commits into from
May 19, 2020
Merged

Pipeline updates #1072

merged 27 commits into from
May 19, 2020

Conversation

jananivMS
Copy link
Contributor

@jananivMS jananivMS commented May 18, 2020 •
edited
Loading

Makes the following updates to the new pipeline and to support it:

  • Fixes the VNET reconciler to report success only after state is "Succeeded". This was causing failures in the MySQL test often.
  • Alters the APIM test to take the pre configured APIM service name as an env var. Without this the tests were failing.
  • Updates the pipeline to also save the helm chart in the helm chart repo
  • Updates the helm chart manifests

@jananivMS jananivMS changed the title [Do not review] Pipeline updates Pipeline updates May 19, 2020
@jananivMS jananivMS requested a review from dkisselev May 19, 2020 13:50
WilliamMortlMicrosoft
WilliamMortlMicrosoft approved these changes May 19, 2020
View reviewed changes
Copy link
Contributor

@WilliamMortlMicrosoft WilliamMortlMicrosoft left a comment

Choose a reason for hiding this comment

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

lgtm

@jananivMS jananivMS merged commit 5af010a into master May 19, 2020
frodopwns
frodopwns reviewed May 19, 2020
View reviewed changes
Copy link
Contributor

@frodopwns frodopwns left a comment

Choose a reason for hiding this comment

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

just the one comment

controllers/apimgmt_controller_test.go
apimServiceName := "AzureOperatorsTestAPIM"

// Read the pre-created APIM service name and RG name from Environment variable if it exists
rgFromEnv := os.Getenv("TEST_APIM_RG")
Copy link
Contributor

Choose a reason for hiding this comment

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

should we just fail id the env vars don't exist? are these hard coded strings correct if the nev vars aren't set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do fall back to a default value for both so we should good for now. But I can open an issue to edit this for later once the old pipeline is retired.

@frodopwns frodopwns deleted the pipeline-updates branch May 19, 2020 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frodopwns frodopwns frodopwns left review comments

@WilliamMortlMicrosoft WilliamMortlMicrosoft WilliamMortlMicrosoft approved these changes

@dkisselev dkisselev dkisselev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jananivMS @frodopwns @dkisselev @WilliamMortlMicrosoft