Skip to content

Update VERSIONS_YAML_FILE e2e test information#916

Merged
istio-testing merged 3 commits intoistio-ecosystem:mainfrom
fjglira:update-docs-versions
Jun 18, 2025
Merged

Update VERSIONS_YAML_FILE e2e test information#916
istio-testing merged 3 commits intoistio-ecosystem:mainfrom
fjglira:update-docs-versions

Conversation

@fjglira
Copy link
Copy Markdown
Contributor

@fjglira fjglira commented Jun 17, 2025

What type of PR is this?

  • Enhancement / New Feature
  • Bug Fix
  • Refactor
  • Optimization
  • Test
  • Documentation Update

What this PR does / why we need it:

Update information related to the VERSIONS_YAML_FILE for running test

Which issue(s) this PR fixes:

Fixes #

Related Issue/PR #

Additional information:

@fjglira fjglira requested a review from a team as a code owner June 17, 2025 09:26
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.93%. Comparing base (72035af) to head (17d013f).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #916   +/-   ##
=======================================
  Coverage   76.93%   76.93%           
=======================================
  Files          44       44           
  Lines        2679     2679           
=======================================
  Hits         2061     2061           
  Misses        524      524           
  Partials       94       94           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@sridhargaddam sridhargaddam left a comment

Choose a reason for hiding this comment

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

@fjglira some tiny nits, otherwise LGTM.

README.md Outdated
#### versions.yaml

The name of the versions.yaml file can be overwritten using the VERSIONS_YAML_FILE environment variable. This way, downstream vendors can point to a custom list of supported versions.
The name of the versions.yaml file can be overwritten using the VERSIONS_YAML_FILE environment variable. This way, downstream vendors can point to a custom list of supported versions. take into account that this file should be located in the `pkg/istioversion` directory, so the default value is `pkg/istioversion/versions.yaml`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

with the default value being pkg/istioversion/versions.yaml
This is a bit confusing, it looks like VERSIONS_YAML_FILE should contain a path but it's not true.
We are using following evn vars to create full path to the file:
VERSIONS_YAML_DIR
VERSIONS_YAML_FILE

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hey @FilipB, but VERSIONS_YAML_DIR is just used in the chart generation and removal of old istio charts, but if you try to run the test with this var, we will get this error, for example:

2025-06-18T10:36:27.306192Z	info	loading supported istio versions from customversion.yaml
panic: failed to read versions from 'customversion.yaml': open customversion.yaml: file does not exist

This is because of the version.go package is using embed to get the version yaml file and is using only the env var VERSIONS_YAML_FILE when it is init

func init() {
	// we can't use ldflags when running tests, use an env variable instead
	// tmp workaround for https://github.com/golang/go/issues/64246
	if versionsFilename == "" {
		versionsFilename = env.Get("VERSIONS_YAML_FILE", "versions.yaml")
	}
	log.Info("loading supported istio versions from " + versionsFilename)
	// Read the embedded versions.yaml file
	data, err := versionsFiles.ReadFile(versionsFilename)
	if err != nil {
		panic(fmt.Errorf("failed to read versions from '%s': %w", versionsFilename, err))
	}

	List, Default, Base, New, Map, aliasList, EOL = mustParseVersionsYaml(data)
}

Go disallows other paths in //go: embed paths, to be able to use it as we need to place it in the same directory as the go function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll rephrase the sentence in the doc to clarify that the file must be located there and we just need to provide the file name

fjglira and others added 2 commits June 18, 2025 08:51
Signed-off-by: Francisco H <frherrer@redhat.com>
Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com>
Signed-off-by: Francisco H <frherrer@redhat.com>
@fjglira fjglira force-pushed the update-docs-versions branch from 433ece3 to 77febd7 Compare June 18, 2025 08:51
Signed-off-by: Francisco H <frherrer@redhat.com>
@fjglira fjglira requested review from FilipB and sridhargaddam June 18, 2025 10:47
@istio-testing istio-testing merged commit 7ff2a24 into istio-ecosystem:main Jun 18, 2025
16 checks passed
openshift-service-mesh-bot pushed a commit to openshift-service-mesh-bot/sail-operator that referenced this pull request Jun 18, 2025
* upstream/main: (34 commits)
  Update VERSIONS_YAML_FILE e2e test information (istio-ecosystem#916)
  Improve automated docs test run (istio-ecosystem#894)
  Use `Eventually` in remote secret generation (istio-ecosystem#921)
  Docs followup link fix (istio-ecosystem#919)
  Fix helm issue during e2e test run  operator installation (istio-ecosystem#914)
  e2e allow to build and push setting target arch (istio-ecosystem#913)
  Fix docs for "broken_links" workflow (istio-ecosystem#918)
  Revert "Disable imageDigest support in ztunnel controller (openshift-service-mesh#582)" (istio-ecosystem#905)
  Fix typo in the ReportAfterSuite text (istio-ecosystem#911)
  Fix cleanup in multicluster tests (istio-ecosystem#909)
  Adding Eventually to Helm operator install (istio-ecosystem#907)
  Skip cleanup if control plane tests fail (istio-ecosystem#910)
  Use debug logging in integration tests (istio-ecosystem#906)
  Annotate only our clusters when using kind (istio-ecosystem#904)
  Fix log message for cleaner when waiting (istio-ecosystem#903)
  Consolidate multicluster operator deploy/cleanup (istio-ecosystem#902)
  Improve e2e test cleanup by recording & cleaning up (istio-ecosystem#889)
  Enchance debug information when e2e fails: add istioctl proxy status (istio-ecosystem#891)
  Update github action for broken links (istio-ecosystem#888)
  Fix "Check broken link" workflow (istio-ecosystem#887)
  ...
@dgn
Copy link
Copy Markdown
Collaborator

dgn commented Jul 3, 2025

/cherry-pick release-1.26

@istio-testing
Copy link
Copy Markdown
Collaborator

@dgn: new pull request created: #981

Details

In response to this:

/cherry-pick release-1.26

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

dgn pushed a commit to dgn/sail-operator that referenced this pull request Mar 17, 2026
* Update VERSIONS_YAML_FILE e2e test information

Signed-off-by: Francisco H <frherrer@redhat.com>

* Apply suggestions from code review

Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com>
Signed-off-by: Francisco H <frherrer@redhat.com>

* Update read me from comments

Signed-off-by: Francisco H <frherrer@redhat.com>

---------

Signed-off-by: Francisco H <frherrer@redhat.com>
Co-authored-by: Sridhar Gaddam <sgaddam@redhat.com>
Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants