search

canonical / dex-auth-operator

Operator for Dex Auth
Apache License 2.0
3 stars 14 forks source link

feat: Remove apt-install from Dex Charm #148

Closed phoevos closed 1 year ago

phoevos commented 1 year ago

Currently, Dex Charm is doing apt install. Update this logic to ensure the Dex Charm can work in an air-gapped environment

Closes https://github.com/canonical/dex-auth-operator/issues/100

kimwnasptd commented 1 year ago

@beliaev-maksim you approved the PR within 4 mins after it was created, and even before the tests were finished.

When did you have the time to test and be sure the PR is OK? 😄

beliaev-maksim commented 1 year ago

@kimwnasptd PR changes are fine. If CI is red then it automatically assumes that it should not be merged :) since not all CI yet executed, you can merge only using "admin" power. The same you can do even without approval :D

phoevos commented 1 year ago

I believe this is what's causing the integration tests to fail:

WARNING  selenium.webdriver.common.selenium_manager:selenium_manager.py:133 The chromedriver version (115.0.5790.110) detected in PATH at /usr/bin/chromedriver might not be compatible with the detected chrome version (115.0.5790.170); currently, chromedriver 115.0.5790.170 is recommended for chrome 115.*, so it is advised to delete the driver in PATH and retry

Because then this is the exception raised:

selenium.common.exceptions.WebDriverException: Message: unknown error: no chrome binary at /opt/google/chrome/google-chrome
beliaev-maksim commented 1 year ago

@phoevos switch to firefox

phoevos commented 1 year ago

I opened an issue for switching to Firefox, I'm not sure at the moment how much effort that requires:

phoevos commented 1 year ago

Looks like the webdriver issue resolved itself, so this is again up for review🎉

kimwnasptd commented 1 year ago

@phoevos @beliaev-maksim to confirm. Do we verify now that the Charm works in airgapped environment since it doesn't do apt-install? Did you try this as well as part of the review?

beliaev-maksim commented 1 year ago

@kimwnasptd I believe airgapped should be tested as separate task, since there could be other pieces that will not work