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

Inherit host's proxy env vars in generated containers #99

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

pastalian
Copy link
Contributor

In Docker CLI, you can configure proxy settings for generated containers
by setting up ~/.docker/config.json. However, Docker provider in
Terraform does not read this file, so proxy env vars need to be
specified directory.

Resolves: AlmaLinux/build-system#321

yum.conf is a symlink to dnf.conf in el8+ but `follow: false` (default)
does not follow the symlink and creates yum.conf as a normal file.
In Docker CLI, you can configure proxy settings for generated containers
by setting up ~/.docker/config.json. However, Docker provider in
Terraform does not read this file, so proxy env vars need to be
specified directory.
@pastalian
Copy link
Contributor Author

I forgot to mention this, but is this line necessary?

dns = ["1.1.1.1", "8.8.8.8"]

I don't think it's a good idea to force specific DNS servers and build.almalinux.org doesn't seem to use it.

@Korulag
Copy link
Contributor

Korulag commented Jul 22, 2024

@pastalian it was the most reliable way to have domain name resolution because sometimes local configuration interferes with it. If you think it's safe to remove it - we can, however I'd like to double check that it will be working in all cases

Copy link

33 passed

Code Coverage Summary

Package Line Rate
alts.scheduler 0%
alts.shared 87%
alts.shared.uploaders 37%
alts.shared.utils 52%
alts.worker 6%
alts.worker.executors 73%
alts.worker.runners 28%
Summary 37% (914 / 2460)

Linter reports

Pylint report
************* Module alts.worker.runners.docker
alts/worker/runners/docker.py:299:0: C0301: Line too long (87/80) (line-too-long)
alts/worker/runners/docker.py:320:0: C0301: Line too long (87/80) (line-too-long)
alts/worker/runners/docker.py:138:4: W0221: Variadics removed in overriding 'DockerRunner.exec_command' method (arguments-differ)
alts/worker/runners/docker.py:318:4: E0102: method already defined line 297 (function-redefined)
alts/worker/runners/docker.py:334:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)

-----------------------------------
Your code has been rated at 8.99/10


Black report
--- alts/worker/runners/docker.py	2024-07-22 12:51:39.044571+00:00
+++ alts/worker/runners/docker.py	2024-07-22 12:53:55.378386+00:00
@@ -162,13 +162,17 @@
         cmd.extend([str(self.env_name), *args])
         self._logger.debug(
             'Running "docker %s" command',
             ' '.join(cmd),
         )
-        return local['docker'].with_cwd(self._work_dir).run(
-            args=tuple(cmd),
-            retcode=None,
+        return (
+            local['docker']
+            .with_cwd(self._work_dir)
+            .run(
+                args=tuple(cmd),
+                retcode=None,
+            )
         )
 
     @staticmethod
     def copy(copy_args: List[str]):
         """
@@ -224,11 +228,12 @@
                     r's/(deb|security)\.debian\.org/archive\.debian\.org/',
                     '/etc/apt/sources.list',
                 )
             self._logger.info('Installing python3 package...')
             exit_code, stdout, stderr = self.exec_command(
-                self.pkg_manager, 'update',
+                self.pkg_manager,
+                'update',
             )
             if exit_code != 0:
                 return exit_code, stdout, stderr
             cmd_args = (self.pkg_manager, 'install', '-y', 'python3')
             exit_code, stdout, stderr = self.exec_command(*cmd_args)
@@ -236,12 +241,19 @@
                 return exit_code, stdout, stderr
             self._logger.info('Installation is completed')
         if self.dist_name in CONFIG.rhel_flavors and self.dist_version == '6':
             self._logger.info('Removing old repositories')
             self.exec_command(
-                'find', '/etc/yum.repos.d', '-type', 'f', '-exec',
-                'rm', '-f', '{}', '+',
+                'find',
+                '/etc/yum.repos.d',
+                '-type',
+                'f',
+                '-exec',
+                'rm',
+                '-f',
+                '{}',
+                '+',
             )
         return super().initial_provision(verbose=verbose)
 
     @command_decorator(
         'package_integrity_tests',
@@ -294,44 +306,56 @@
         'Third party tests failed',
         exception_class=ThirdPartyTestError,
     )
     def run_third_party_test(
         self,
-        executor: Union[AnsibleExecutor, BatsExecutor, CommandExecutor, ShellExecutor],
+        executor: Union[
+            AnsibleExecutor, BatsExecutor, CommandExecutor, ShellExecutor
+        ],
         cmd_args: List[str],
         docker_args: Optional[List[str]] = None,
         workdir: str = '',
         artifacts_key: str = '',
         additional_section_name: str = '',
         env_vars: Optional[List[str]] = None,
     ):
-        return executor.run_docker_command(
-            cmd_args=cmd_args,
-            docker_args=docker_args,
-            env_vars=env_vars,
-        ).model_dump().values()
+        return (
+            executor.run_docker_command(
+                cmd_args=cmd_args,
+                docker_args=docker_args,
+                env_vars=env_vars,
+            )
+            .model_dump()
+            .values()
+        )
 
     @command_decorator(
         '',
         'Third party tests failed',
         exception_class=ThirdPartyTestError,
     )
     def run_third_party_test(
         self,
-        executor: Union[AnsibleExecutor, BatsExecutor, CommandExecutor, ShellExecutor],
+        executor: Union[
+            AnsibleExecutor, BatsExecutor, CommandExecutor, ShellExecutor
+        ],
         cmd_args: List[str],
         docker_args: Optional[List[str]] = None,
         workdir: str = '',
         artifacts_key: str = '',
         additional_section_name: str = '',
         env_vars: Optional[List[str]] = None,
     ):
-        return executor.run_docker_command(
-            cmd_args=cmd_args,
-            docker_args=docker_args,
-            env_vars=env_vars,
-        ).model_dump().values()
+        return (
+            executor.run_docker_command(
+                cmd_args=cmd_args,
+                docker_args=docker_args,
+                env_vars=env_vars,
+            )
+            .model_dump()
+            .values()
+        )
 
     def clone_third_party_repo(
         self,
         repo_url: str,
         git_ref: str,
@@ -352,15 +376,18 @@
         'stop_environment',
         'Cannot destroy environment',
         exception_class=StopEnvironmentError,
     )
     def stop_env(self):
-        _, container_id, _ = local['terraform'].with_cwd(
-            self._work_dir).run(
-            args=('output', '-raw', '-no-color', 'container_id'),
-            retcode=None,
-            timeout=CONFIG.provision_timeout,
+        _, container_id, _ = (
+            local['terraform']
+            .with_cwd(self._work_dir)
+            .run(
+                args=('output', '-raw', '-no-color', 'container_id'),
+                retcode=None,
+                timeout=CONFIG.provision_timeout,
+            )
         )
         try:
             return super().stop_env()
         except StopEnvironmentError:
             # Attempt to delete environment via plain docker command

Isort report
--- /code/alts/worker/runners/docker.py:before	2024-07-22 12:51:39.044571
+++ /code/alts/worker/runners/docker.py:after	2024-07-22 12:53:55.980946
@@ -10,8 +10,8 @@
     Callable,
     List,
     Optional,
+    Tuple,
     Union,
-    Tuple,
 )
 
 from plumbum import local

Bandit report
Run started:2024-07-22 12:53:56.645721

Test results:
	No issues identified.

Code scanned:
	Total lines of code: 327
	Total lines skipped (#nosec): 0
	Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 0
	Total issues (by confidence):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 0
Files skipped (0):

View full reports on the Job Summary page.

@pastalian
Copy link
Contributor Author

I see. Then I'll leave it as is for now. I can simply remove it on my dev machine.

@Korulag Korulag merged commit b23b5fa into AlmaLinux:master Jul 22, 2024
3 checks passed
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.

proxy support for alts
2 participants