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.

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

Fix RedHat urls #72

Merged
merged 4 commits into from
Nov 23, 2023
Merged

Fix RedHat urls #72

merged 4 commits into from
Nov 23, 2023

Conversation

GMZwinge
Copy link
Contributor

@GMZwinge GMZwinge commented Nov 21, 2023

Pull Request (PR) description

Under https://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/, the urls have moved from CentOS_9 to CentOS_9_Stream. Eg: https://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/CentOS_9_Stream/ and https://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable:/cri-o:/1.26:/1.26.1/CentOS_9_Stream/.

This Pull Request (PR) fixes the following issues

When applying to a CentOS 9 machine.

Copy link
Member

@ananace ananace left a comment

Choose a reason for hiding this comment

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

This would break EL7 support, which still has a while left until it's EoL.

My suggestion is to only add _Stream if the major is larger than 7.

manifests/repo.pp Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member

@GMZwinge can you please add a unit test?

Copy link
Member

@tuxmea tuxmea left a comment

Choose a reason for hiding this comment

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

pleaae add this to a unit test

@GMZwinge
Copy link
Contributor Author

@GMZwinge can you please add a unit test?

@bastelfreak Not sure what you mean. It looks like there is already a unit test.

@tuxmea
Copy link
Member

tuxmea commented Nov 22, 2023

You want to add spec test to check that the correct repo is taken:

Example (not rubocop clean):

  on_supported_os.each do |os, os_facts|
    context "on #{os}" do
      let(:facts) { os_facts }

      it { is_expected.to compile }
    end
    context "on CentOS 7 and 8" do
      let(:facts) { os_facts }
      if os['name'] == 'CentOS' and os['release']['major'] == '7'
        it { is_expected.to contain_yumrepo('libcontainers:stable').with_baseurl => 'https://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/CentOS_7}/
      end
      if os['name'] == 'CentOS' and os['release']['major'] == '8'
        it { is_expected.to contain_yumrepo('libcontainers:stable').with_baseurl => 'https://download.opensuse.org/repositories/devel:/kubic:/libcontainers:/stable/CentOS_Stream_8}/
      end
    end
  end

I suppose that there are more elegant ways on how to test behavior.

@GMZwinge
Copy link
Contributor Author

@tuxmea @bastelfreak: unit test added.

@tuxmea tuxmea merged commit b736d35 into voxpupuli:master Nov 23, 2023
3 checks passed
@bastelfreak bastelfreak added the bug Something isn't working label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants