Skip to content

Conversation

kcrocombe
Copy link

Existing integration test cases for farm functionality is limited to the operations that create/update/delete farm objects. There is no test coverage of any actual farm builds.

This submission seeks to address this via the addition of a new integration test module: farm_build_test.go

This module:

  • prepares a simulated, multi-farm, multi-node test environment;
  • performs testing of the podman farm build function across that environment;
  • confirms that the expected number of build are performed; that they are of the correct architecture; and that the builds occur on the expected node, given the build parameters that were supplied.

The commit also includes two minor changes:

  • A very minor change the farm documention to clarify one of of the default setting.

  • The farm system-test script 001-farm.bats now uses an explicitly set connection name rather than relying on the default in the users environment being set in a supportive manner.

Does this PR introduce a user-facing change?

None

Existing integration test cases for `farm` functionality
is limited to the operations that create/update/delete farm
objects. There is no test coverage of any actual farm builds.

This submission seeks to address this via the addition of
a new integration test module: `farm_build_test.go`

This module:
  - prepares a simulated, multi-farm, multi-node test environment;
  - performs testing of the podman farm build function across
    that environment;
  - confirms that the expected number of build are performed; that
    they are of the correct architecture; and that the builds occur
    on the expected node, given the build parameters that were supplied.

The commit also includes two minor changes:

  - A very minor change the farm documention to clarify one of
    of the `default` setting.

  - The farm system-test script 001-farm.bats now uses an
    explicitly set connection name rather than relying on
    the default in the users environment being set in a
    supportive manner.

Signed-off-by: kevin <[email protected]>
@kcrocombe
Copy link
Author

/test ?

Copy link
Contributor

openshift-ci bot commented Sep 3, 2025

@kcrocombe: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test ?

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.

@kcrocombe
Copy link
Author

/retest

Copy link
Contributor

openshift-ci bot commented Sep 3, 2025

@kcrocombe: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

@kcrocombe
Copy link
Author

kcrocombe commented Sep 3, 2025

@containers/podman-maintainers: Is it possible to re-run the above failing checks please? Can't draw a line between the test failures and anything in my PR, so want to rule out flakiness. Also is it possible for me to run re-run these myself? I couldn't work out how to.

@baude
Copy link
Member

baude commented Sep 3, 2025

it is not possible to rerun yourself and it is quite likely a flake. ive rerun the test now and will try to keep an eye on it. thanks for the submission.

@kcrocombe
Copy link
Author

Thanks for doing that. Though with the same result unfortunately. The failing test seems to think an additional google nameserver has been introduced to host/resolv.conf that it wasn't expecting to see. Still struggling to see why a changes to a test-case and a bit of documentation would be responsible for that, though.

@kcrocombe
Copy link
Author

Spoke too soon...thanks for sorting that out for me. Much appreciated.

@mheon
Copy link
Member

mheon commented Sep 3, 2025

The general approach here (reverse-proxy to simulate multiple nodes) is inspired - nice work!
/approve

Copy link
Contributor

openshift-ci bot commented Sep 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kcrocombe, mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 3, 2025
@kcrocombe
Copy link
Author

The general approach here (reverse-proxy to simulate multiple nodes) is inspired - nice work! /approve

@mheon Yeah, I thought so too; which is why I stole the idea, without shame, from whoever wrote system_connection_test.go (@ben-krieger possibly). I thank you on his behalf.

@kcrocombe
Copy link
Author

@baude : This may have skipped ahead of it's self. Though approved, I understand it still needs a second pair of eyes to move forward. Could you have look when you've got a moment?. Ta.

@kcrocombe
Copy link
Author

Losing a bit of traction on this, I think, and its now backing in to the couple of PR's I have to follow on. To keep things moving, I suggest I now pull this PR and wrap its contents into its successor? Please advise me otherwise if this isn't the best plan. Ta.

@mheon
Copy link
Member

mheon commented Sep 8, 2025

I'm good to merge as-is.
LGTM
@containers/podman-maintainers PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants