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

Add ftp-proxy test #1281

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add ftp-proxy test #1281

wants to merge 3 commits into from

Conversation

jkonecny12
Copy link
Member

@jkonecny12 jkonecny12 commented Aug 12, 2024 •
edited
Loading

Add a new test ftp-proxy which will use proxy server together with FTP installation source.

This PR is also:

  • introducing a new ftp test group
  • fixing validation of our shared proxy check even that it's not used here because it would be hard to decide this is FTP and those checks there won't be that helpful.

This PR is blocked by a lot of fixes:

@jkonecny12 jkonecny12 added the blocked This can't be merged before dependencies! label Aug 12, 2024
This was referenced Aug 12, 2024
Merged
Merged
Merged
M4rtinK
M4rtinK approved these changes Aug 13, 2024
View reviewed changes
Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Maybe you could use fragment/s for the generic part of the kickstart file ? Otherwise looks good. :)

ftp-proxy.ks.in
network --bootproto=dhcp

bootloader --timeout=1
zerombr
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if using some fragments could prevent duplication ? For example, I don't think the storage section is specific to this test & could be replaced by: https://github.com/rhinstaller/kickstart-tests/blob/master/fragments/shared/storage/default.ks

Or possibly the full common_no_payload ?
https://github.com/rhinstaller/kickstart-tests/blob/master/fragments/shared/common/common_no_payload.ks

Copy link
Member Author

Choose a reason for hiding this comment

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

This is taken from the ftp test. Let's use it this way and I can take a look on the optimization of both in next PR.

@jkonecny12
Copy link
Member Author

UPDATED:

  • Rebased on top of other dependencies
  • Resolve one conflict

@jkonecny12
Copy link
Member Author

Rebased to resolve smaller conflict.

@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

/test-os-variants

@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

/test-os-variants

@jkonecny12
Copy link
Member Author

/test-tmt

1 similar comment
@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

/test-tmt

3 similar comments
@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12 jkonecny12 mentioned this pull request Nov 19, 2024
Merged
@jkonecny12
Fix .treeinfo check in the functions-proxy utils
5bf8f4a 
Squid prints file name in the middle of the line not on the end. Maybe
change in the squid logging?
@jkonecny12
Create a new test group 'ftp'
b723c23 
We have only one now but next one is coming.
@jkonecny12
Add a new test ftp-proxy
cbe443b 
Test if FTP installation with `inst.proxy` works as expected.

To be able to support this we need to allow more ports in the squid.
@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

Rebasing and rerunning to test out change from #1340

@jkonecny12
Copy link
Member Author

/test-tmt

@jkonecny12
Copy link
Member Author

No it doesn't helped :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@M4rtinK M4rtinK M4rtinK approved these changes

Assignees
No one assigned
Labels
blocked This can't be merged before dependencies!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jkonecny12 @M4rtinK