engineering: use correct version of IC for rollback tests#517
engineering: use correct version of IC for rollback tests#517
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the rollback testing pipeline to use a configurable Image Customizer (IC/MIC) version instead of defaulting to :latest. The changes enable the pipeline's micVersion and micBuildType parameters to control which IC version is used during rollback tests, consistent with how other testing pipelines operate.
Changes:
- Add
micBuildTypeandmicVersionparameter passing from vm-testing.yml to all three testing-template.yml invocations - Add
micBuildTypeandmicVersionparameter definitions in testing-template.yml - Add MIC download template calls and dev build type logic to use the specified IC version
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .pipelines/templates/stages/testing_rollback/vm-testing.yml | Passes micBuildType and micVersion parameters to all three testing-template.yml invocations (qemu-grub, qemu, and uki flavors) |
| .pipelines/templates/stages/testing_rollback/testing-template.yml | Adds micBuildType and micVersion parameter definitions, MIC download template calls, and logic to use dev IC image when micBuildType is 'dev' |
| - template: common/mic-download-template.yaml@platform-pipelines | ||
| parameters: | ||
| buildType: ${{ parameters.micBuildType }} | ||
| arch: ${{ parameters.micArchitecture }} |
There was a problem hiding this comment.
The micArchitecture parameter is referenced in the mic-download-template calls but is not defined in the parameters section of this template. Add a parameter definition for micArchitecture (similar to trident-testimg-template.yml lines 18-24) with a default value of "amd64" to match the hostArchitecture specified in line 77 of this file.
Changed architecture parameter to 'amd64' in mic-download-template.
| // Check for Image Customizer image | ||
| queryImageArgs := []string{"images", "-q", testConfig.ImageCustomizerImage} | ||
| logrus.Tracef("Checking for Image Customizer image: %v", queryImageArgs) | ||
| queryImageOutput, err := exec.Command("docker", queryImageArgs...).Output() | ||
| logrus.Tracef("Check for Image Customizer image (%v):\n%s", err, string(queryImageOutput)) | ||
| if err != nil || len(queryImageOutput) == 0 { | ||
| logrus.Tracef("Image Customizer image not found locally: %s", testConfig.ImageCustomizerImage) | ||
| // Pull Image Customizer image | ||
| pullArgs := []string{"pull", testConfig.ImageCustomizerImage} | ||
| logrus.Tracef("Pulling Image Customizer image: %v", pullArgs) | ||
| pullOutput, err := exec.Command("docker", pullArgs...).CombinedOutput() | ||
| logrus.Tracef("Pull Image Customizer (%v):\n%s", err, string(pullOutput)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to pull Image Customizer image: %w", err) | ||
| } | ||
| logrus.Tracef("Pulled Image Customizer image: %s", testConfig.ImageCustomizerImage) | ||
| } |
There was a problem hiding this comment.
Issue: the new “check locally then pull” logic can cause rollback tests to run against a stale Image Customizer image when the tag is mutable (e.g. :latest default or :dev). This undermines the goal of pinning/using the intended IC version.
Evidence: PrepareQcow2 only pulls when docker images -q returns empty, and later runs with docker run --pull=never.
Suggestion: either always docker pull the requested tag (relying on Docker layer caching), or conditionally force pull for mutable tags (latest/dev) / add a “force pull” option; also consider trimming docker images -q output and capturing stderr for easier diagnostics.
| // Check for Image Customizer image | |
| queryImageArgs := []string{"images", "-q", testConfig.ImageCustomizerImage} | |
| logrus.Tracef("Checking for Image Customizer image: %v", queryImageArgs) | |
| queryImageOutput, err := exec.Command("docker", queryImageArgs...).Output() | |
| logrus.Tracef("Check for Image Customizer image (%v):\n%s", err, string(queryImageOutput)) | |
| if err != nil || len(queryImageOutput) == 0 { | |
| logrus.Tracef("Image Customizer image not found locally: %s", testConfig.ImageCustomizerImage) | |
| // Pull Image Customizer image | |
| pullArgs := []string{"pull", testConfig.ImageCustomizerImage} | |
| logrus.Tracef("Pulling Image Customizer image: %v", pullArgs) | |
| pullOutput, err := exec.Command("docker", pullArgs...).CombinedOutput() | |
| logrus.Tracef("Pull Image Customizer (%v):\n%s", err, string(pullOutput)) | |
| if err != nil { | |
| return fmt.Errorf("failed to pull Image Customizer image: %w", err) | |
| } | |
| logrus.Tracef("Pulled Image Customizer image: %s", testConfig.ImageCustomizerImage) | |
| } | |
| // Always pull the Image Customizer image to avoid using a stale mutable tag. | |
| pullArgs := []string{"pull", testConfig.ImageCustomizerImage} | |
| logrus.Tracef("Pulling Image Customizer image: %v", pullArgs) | |
| pullOutput, err := exec.Command("docker", pullArgs...).CombinedOutput() | |
| logrus.Tracef("Pull Image Customizer (%v):\n%s", err, string(pullOutput)) | |
| if err != nil { | |
| return fmt.Errorf("failed to pull Image Customizer image: %w", err) | |
| } | |
| logrus.Tracef("Pulled Image Customizer image: %s", testConfig.ImageCustomizerImage) |
| skipNetplanRuntimeTesting: true | ||
| micBuildType: ${{ parameters.micBuildType }} | ||
| micVersion: ${{ parameters.micVersion }} | ||
| isUki: true |
There was a problem hiding this comment.
Issue: isUki: true is set for the flavor: qemu rollback job even though that job is not the UKI flavor. This makes the template parameters misleading and risks accidentally enabling UKI-specific behavior if skipExtensionTesting changes later.
Evidence: flavor: qemu block passes isUki: true.
Suggestion: only set isUki: true for the flavor: uki job (or rename the parameter if it’s intended to mean something else).
| isUki: true |
3 participants
🔍 Description
The rollback tests are using
:latestIC. Enable the pipeline micVersion and micBuildType to determine IC version as it does elsewhere.