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

cockpit: fix listen port (#371245) #374667

Merged
merged 2 commits into from
Jan 18, 2025
Merged

cockpit: fix listen port (#371245) #374667

merged 2 commits into from
Jan 18, 2025
+7 −5

Conversation

elratt0r
Copy link
Contributor

@elratt0r elratt0r commented Jan 17, 2025 •
edited by JohnRTitor
Loading

Closes #371245

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 17, 2025
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jan 17, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 17, 2025
@elratt0r elratt0r mentioned this pull request Jan 17, 2025
Closed
@lucasew
Copy link
Contributor

lucasew commented Jan 17, 2025

A GitHub tip there. If you add a Closes #issue it will automatically close the issue when the PR is merged.

Tools that lazy people like me appreciate lol

@lucasew
Copy link
Contributor

lucasew commented Jan 17, 2025

If you don't mind, can you change the port for the test and setup the client part to use that port?

@lucasew
Copy link
Contributor

lucasew commented Jan 18, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374667

x86_64-linux

✅ 1 test built:
  • nixosTests.cockpit

@lucasew lucasew requested a review from GaetanLepage January 18, 2025 01:15
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374667

x86_64-linux

⏩ 2 packages blacklisted:
  • nixos-install-tools
  • tests.nixos-functions.nixos-test

aarch64-linux

⏩ 2 packages blacklisted:
  • nixos-install-tools
  • tests.nixos-functions.nixos-test

GaetanLepage
GaetanLepage approved these changes Jan 18, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

LGTM

@JohnRTitor
Copy link
Contributor

@GaetanLepage Sorry I don't know much about this module.

@JohnRTitor JohnRTitor requested review from SuperSandro2000 and removed request for JohnRTitor January 18, 2025 14:52
@lucasew
Copy link
Contributor

lucasew commented Jan 18, 2025 •
edited
Loading

@elratt0r did it right.

Cockpit wasnt respecting the port option, now it is, and the problem with the other approach is that it listened on both 9090 and the selected port, which you solved and setup the test to check it.

I think this is ready for merge.

@GaetanLepage GaetanLepage merged commit b60bec4 into NixOS:master Jan 18, 2025
34 of 36 checks passed
@GaetanLepage
Copy link
Contributor

@GaetanLepage Sorry I don't know much about this module.

No worry, it was more for double-checking. I'm confident that this change is ready.

@lucasew lucasew added the backport release-24.11 Backport PR automatically label Jan 18, 2025
@nixpkgs-ci
Copy link

nixpkgs-ci bot commented Jan 18, 2025

Successfully created backport PR for release-24.11:

@nixpkgs-ci nixpkgs-ci bot mentioned this pull request Jan 18, 2025
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaetanLepage GaetanLepage GaetanLepage approved these changes

@lucasew lucasew lucasew approved these changes

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12. first-time contribution This PR is the author's first one; please be gentle! backport release-24.11 Backport PR automatically
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cockpit parameter services.cockpit.port is not honored anymore
5 participants
@elratt0r @lucasew @GaetanLepage @JohnRTitor @NixOSInfra