diff --git a/hotfix/hotfix_generate.py b/hotfix/hotfix_generate.py index 2623604bac3..432c96eb0ae 100644 --- a/hotfix/hotfix_generate.py +++ b/hotfix/hotfix_generate.py @@ -12,10 +12,10 @@ 2. Detects which CSE provisioning scripts differ from the immutable VHD baseline (the release tag the VHD was built from, derived from linux_sig_version.json), selects their write_files entries from parts/linux/cloud-init/nodecustomdata.yml, - injects those entries into the scriptless section of nodecustomdata.yml, and - renders self-contained ANC payloads for Ubuntu and Mariner with AgentBaker's - canonical Go-template renderer. Diffing against the frozen baseline keeps both - delivery paths cumulative. + injects entries not already supplied by the scriptless section into + nodecustomdata.yml, and renders self-contained ANC payloads for Ubuntu and + Mariner with AgentBaker's canonical Go-template renderer. Diffing against the + frozen baseline keeps both delivery paths cumulative. 3. Writes `scripts_version` for script hotfixes. With --use-anc-for-scripts, also writes the `hotfixes` map to activate the embedded ANC payload. Independent ANC @@ -467,7 +467,7 @@ def update_nodecustomdata(target_varkeys): return lines = clean_content.splitlines(keepends=True) - _, else_line, end_line = find_block_boundaries(lines) + start_line, else_line, end_line = find_block_boundaries(lines) if else_line is None or end_line is None: raise GenerationError( f"could not find traditional write_files section in {TEMPLATE}" @@ -480,10 +480,23 @@ def update_nodecustomdata(target_varkeys): if not selected_blocks: raise GenerationError("no matching write_files blocks found for nodecustomdata") - injected_lines = ["\n", f"{SCRIPTS_BEGIN}\n"] - for block_lines in selected_blocks: - injected_lines.extend(block_lines) - injected_lines.append(f"{SCRIPTS_END}\n") + # Compare entire blocks, including guards and content, ignoring only boundary + # whitespace. Matching paths or variable keys alone can hide distinct entries. + existing_blocks = { + "".join(block_lines).strip() + for _, block_lines in parse_write_files_blocks(lines[start_line + 1:else_line]) + } + selected_blocks = [ + block_lines for block_lines in selected_blocks + if "".join(block_lines).strip() not in existing_blocks + ] + + injected_lines = [] + if selected_blocks: + injected_lines = ["\n", f"{SCRIPTS_BEGIN}\n"] + for block_lines in selected_blocks: + injected_lines.extend(block_lines) + injected_lines.append(f"{SCRIPTS_END}\n") with open(TEMPLATE, "w") as template_file: template_file.writelines(lines[:else_line] + injected_lines + lines[else_line:]) diff --git a/hotfix/hotfix_generate_test.py b/hotfix/hotfix_generate_test.py index 23b5ab5b11e..3afae8998e8 100644 --- a/hotfix/hotfix_generate_test.py +++ b/hotfix/hotfix_generate_test.py @@ -377,6 +377,100 @@ def test_update_nodecustomdata_injects_and_removes_selected_blocks(self): self.assertNotIn(hotfix_generate.SCRIPTS_BEGIN, cleaned) self.assertEqual(1, cleaned.count("provisionSourceUbuntu")) + def test_update_nodecustomdata_deduplicates_existing_scriptless_block(self): + repository = Path(__file__).resolve().parents[1] + original = (repository / hotfix_generate.TEMPLATE).read_text() + lines = original.splitlines(keepends=True) + start, outer_else, end = hotfix_generate.find_block_boundaries(lines) + init_block = next( + "".join(block).strip() + "\n" + for keys, block in hotfix_generate.parse_write_files_blocks( + lines[outer_else + 1:end] + ) + if keys == {"initAKSCloud"} + ) + self.assertIn(init_block, "".join(lines[start + 1:outer_else])) + + for selected in ({"initAKSCloud"}, {"initAKSCloud", "provisionSource"}): + with self.subTest(selected=selected), tempfile.TemporaryDirectory() as temp_dir: + template = Path(temp_dir) / "nodecustomdata.yml" + template.write_text( + "".join(lines[:outer_else]) + + f"\n{hotfix_generate.SCRIPTS_BEGIN}\n" + + init_block + + f"{hotfix_generate.SCRIPTS_END}\n" + + "".join(lines[outer_else:]) + ) + with mock.patch.object(hotfix_generate, "TEMPLATE", str(template)): + hotfix_generate.update_nodecustomdata(selected) + updated = template.read_text() + updated_lines = updated.splitlines(keepends=True) + _, updated_else, updated_end = hotfix_generate.find_block_boundaries( + updated_lines + ) + self.assertEqual( + lines[outer_else + 1:end], + updated_lines[updated_else + 1:updated_end], + ) + self.assertEqual(2, updated.count(init_block.strip())) + if "provisionSource" in selected: + generated = updated.split(hotfix_generate.SCRIPTS_BEGIN, 1)[1] + generated = generated.split(hotfix_generate.SCRIPTS_END, 1)[0] + self.assertNotIn("initAKSCloud", generated) + self.assertIn('"provisionSource"', generated) + else: + self.assertNotIn(hotfix_generate.SCRIPTS_BEGIN, updated) + + hotfix_generate.update_nodecustomdata(selected) + self.assertEqual(updated, template.read_text()) + + hotfix_generate.update_nodecustomdata(set()) + self.assertEqual(original, template.read_text()) + hotfix_generate.update_nodecustomdata(set()) + self.assertEqual(original, template.read_text()) + + def test_update_nodecustomdata_keeps_differently_defined_blocks(self): + block = ( + "{{if IsAKSCustomCloud}}\n" + "- path: {{GetInitAKSCloudFilepath}}\n" + ' permissions: "0744"\n' + " encoding: gzip\n" + " owner: root\n" + " content: !!binary |\n" + ' {{GetVariableProperty "cloudInitData" "initAKSCloud"}}\n' + "{{end}}\n" + ) + variants = ( + block.replace("IsAKSCustomCloud", "IsMariner"), + "{{if IsMariner}}\n" + block + "{{end}}\n", + block.replace("{{if IsAKSCustomCloud}}\n", "").removesuffix("{{end}}\n"), + block.replace('"0744"', '"0644"'), + block.replace("owner: root", "owner: other"), + block.replace("encoding: gzip", "encoding: b64"), + block.replace("GetInitAKSCloudFilepath", "GetCSEHelpersScriptFilepath"), + block.replace('"initAKSCloud"', '"provisionSource"'), + ) + for existing in variants: + with self.subTest(existing=existing), tempfile.TemporaryDirectory() as temp_dir: + template = Path(temp_dir) / "nodecustomdata.yml" + template.write_text( + "#cloud-config\nwrite_files:\n" + "{{if EnableScriptlessCSECmd}}\n" + + existing + + "{{- else }}\n" + + block + + "{{- end }}\n" + ) + with mock.patch.object(hotfix_generate, "TEMPLATE", str(template)): + hotfix_generate.update_nodecustomdata({"initAKSCloud"}) + updated = template.read_text() + generated = updated.split(hotfix_generate.SCRIPTS_BEGIN, 1)[1] + generated = generated.split(hotfix_generate.SCRIPTS_END, 1)[0] + self.assertEqual(block.strip(), generated.strip()) + self.assertIn(existing, updated) + hotfix_generate.update_nodecustomdata({"initAKSCloud"}) + self.assertEqual(updated, template.read_text()) + def test_write_hotfix_file_uses_hotfixes_and_preserves_scripts_version(self): with tempfile.TemporaryDirectory() as temp_dir: target = Path(temp_dir) / "hotfix.json"