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

install: /usr/sbin/transactional-update (and uninstall fix) #4403

Merged

Conversation

dweomer
Copy link
Contributor

@dweomer dweomer commented Nov 5, 2021

Proposed Changes

Fix for sle-micro / microos to use the correct location when attempting to detect for transactional-update

Types of Changes

installer script

Verification

cd tests/install/opensuse-microos
vagrant plugin install vagrant-reload vagrant-k3s
vagrant up --provider=virtualbox

Linked Issues

#4402
#4409

User-Facing Change

Improved MicroOS support for k3s-uninstall.sh

Further Comments

@dweomer dweomer requested a review from a team as a code owner November 5, 2021 06:08
@dweomer dweomer force-pushed the fix/4402/usr/sbin/transactional-update branch from 71b2ab2 to 03ed472 Compare November 5, 2021 15:41
@dweomer dweomer changed the title install: /usr/sbin/transactional-update install: /usr/sbin/transactional-update (and uninstall fix) Nov 5, 2021
- also updated k3s-uninstall.sh on zypper and TU systems
- fix k3s-io#4409 for Fedora CoreOS

new installer tests via github actions:
- fedora-coreos
- opensuse-microos

Signed-off-by: Jacob Blain Christen <jacob@rancher.com>
'vagrant-reload' => {},
}

config.vm.define 'install-microos', primary: true do |test|
test.vm.box = 'opensuse/MicroOS.x86_64'
test.vm.box = 'dweomer/microos.amd64'
Copy link
Contributor

Choose a reason for hiding this comment

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

why change this?

Copy link
Contributor Author

@dweomer dweomer Nov 8, 2021

Choose a reason for hiding this comment

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

It makes the tests faster as the box that I have built has selinux enabled already and is available for the virtualbox provider (which means we can actually exercise it via github actions automation). Additionally, this box has a /vagrant subvolume setup that is owned by the vagrant user, making it more amenable to prevailing Vagrantisms (read: idiomatic expectations).

Copy link
Contributor

Choose a reason for hiding this comment

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

It makes the tests faster as the box that I have built has selinux enabled already and is available for the virtualbox provider (which means we can actually exercise it via github actions automation). Additionally, this box has a /vagrant subvolume setup that is owned by the vagrant user, making it more amenable to prevailing Vagrantisms (read: idiomatic expectations).

All good things … so why not contribute them to the official MicroOS image rather than use your own which (practically speaking) isn’t actually MicroOS but a derivative?

I’d happily accept changes to the vagrant definition in https://build.opensuse.org/package/view_file/openSUSE:Factory/openSUSE-MicroOS/openSUSE-MicroOS.kiwi?expand=1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @sysrich, I do appreciate you reaching out with the nudge and the link. I have refrained from such thus far for two not insurmountable reasons:

  1. Navigating the OBS space is kind of daunting for the uninitiated. I have a rant about this that I would very much like to be proven wrong on. Or maybe you've got some pointers. Would like to chat sometime. (Feels kinda like being a developer using a Mac: "I know what I am doing, get all of this shiny stuff out of my way." Which isn't to say there is nothing for me to learn, because I know there is.)
  2. I assume that MicroOS is packaged/built the way it is for very good reasons and that likely doesn't need to change. A variant Vagrant box, however, may be in order. Thorsten likely has some opinions on this as well. Worth discussing!

Additionally, as a k3s maintainer, I think we can do a better job of packaging and installation on MicroOS. There are some philosophical differences, however, to at least recognize, if not wholly overcome, before improvement can be made (IMHO), and so I have avoided touching on any openSUSE (or SUSE) space where my presence or that of anyone else on the team could be perceived as combative or overly critical. I would like to help effect positive change and so am taking care with my actions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressing 1. - ‘osc bco openSUSE-MicroOS’ is equivalent to GitHub forking and clone in one easy command. Make your changes, ‘osc vc’ to write a change log, ‘osc ci’ to check in your branch and ‘osc sr’ to send it for review - just like GitHub but easier as I don’t need to tell you how to nav around a Webui :)

  1. you use it, you have a right to help shape it. MicroOS exists to be a good platform for things like k3s so we better adapt to your needs. And I’d rather deal with that than bug reports we get because your derivative lags behind our images (which it always will :))

Happy to hear any philosophical differences you perceive, I think you’ll find things in MicroOS land are more malleable than you might expect :) look forward to hearing from you

@dereknola
Copy link
Contributor

Can you add a blurb in https://github.com/k3s-io/k3s/blob/master/tests/TESTING.md about what the tests/install subfolder of tests does.

@dweomer
Copy link
Contributor Author

dweomer commented Nov 8, 2021

Can you add a blurb in https://github.com/k3s-io/k3s/blob/master/tests/TESTING.md about what the tests/install subfolder of tests does.

@dereknola as per the very recent call we just had, I will do this in a follow-up PR (and move stuff into a vagrant folder).

@dweomer dweomer merged commit 559c8ad into k3s-io:master Nov 8, 2021
@dweomer dweomer deleted the fix/4402/usr/sbin/transactional-update branch November 8, 2021 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants