Merge pull request #9119 from mgugino-upstream-stage/cleanup-todos

Fixup various TODO sections of code

playbooks/init/basic_facts.yml

@@ -87,14 +87,6 @@
         osm_host_subnet_length: "{{ l_existing_config_master_config.networkConfig.clusterNetworks[0].hostSubnetLength }}"
       when:
       - l_existing_config_master_config.networkConfig.clusterNetworks is defined
-
-    # TODO(michaelgugino): Remove in 3.11
-    # Use deprecated config variables if newer ones not found above.
-    - set_fact:
-        osm_cluster_network_cidr: "{{ l_existing_config_master_config.networkConfig.clusterNetworkCIDR }}"
-        osm_host_subnet_length: "{{ l_existing_config_master_config.networkConfig.hostSubnetLength }}"
-      when:
-      - l_existing_config_master_config.networkConfig.clusterNetworks is not defined
       # End block
 
 - name: Initialize special first-master variables

roles/lib_utils/action_plugins/sanity_checks.py

@@ -63,53 +63,8 @@ IMAGE_POLICY_CONFIG_VAR = "openshift_master_image_policy_config"
 ALLOWED_REGISTRIES_VAR = "openshift_master_image_policy_allowed_registries_for_import"
 
 REMOVED_VARIABLES = (
-    # TODO(michaelgugino): Remove these in 3.11
-    ('openshift_metrics_image_prefix', 'openshift_metrics_<component>_image'),
-    ('openshift_metrics_image_version', 'openshift_metrics_<component>_image'),
-    ('openshift_grafana_proxy_image_prefix', 'openshift_grafana_proxy_image'),
-    ('openshift_grafana_proxy_image_version', 'openshift_grafana_proxy_image'),
-    ('openshift_logging_image_prefix', 'openshift_logging_image'),
-    ('openshift_logging_image_verion', 'openshift_logging_image'),
-    ('openshift_logging_curator_image_prefix', 'openshift_logging_curator_image'),
-    ('openshift_logging_curator_image_version', 'openshift_logging_curator_image'),
-    ('openshift_logging_elasticsearch_image_prefix', 'openshift_logging_elasticsearch_image'),
-    ('openshift_logging_elasticsearch_image_version', 'openshift_logging_elasticsearch_image'),
-    ('openshift_logging_elasticsearch_proxy_image_prefix', 'openshift_logging_elasticsearch_proxy_image'),
-    ('openshift_logging_elasticsearch_proxy_image_version', 'openshift_logging_elasticsearch_proxy_image'),
-    ('openshift_logging_fluentd_image_prefix', 'openshift_logging_fluentd_image'),
-    ('openshift_logging_fluentd_image_version', 'openshift_logging_fluentd_image'),
-    ('openshift_logging_kibana_image_prefix', 'openshift_logging_kibana_image'),
-    ('openshift_logging_kibana_image_version', 'openshift_logging_kibana_image'),
-    ('openshift_logging_kibana_proxy_image_prefix', 'openshift_logging_kibana_proxy_image'),
-    ('openshift_logging_kibana_proxy_image_version', 'openshift_logging_kibana_proxy_image'),
-    ('openshift_logging_mux_image_prefix', 'openshift_logging_mux_image'),
-    ('openshift_logging_mux_image_version', 'openshift_logging_mux_image'),
-    ('openshift_prometheus_image_prefix', 'openshift_prometheus_image'),
-    ('openshift_prometheus_image_version', 'openshift_prometheus_image'),
-    ('openshift_prometheus_proxy_image_prefix', 'openshift_prometheus_proxy_image'),
-    ('openshift_prometheus_proxy_image_version', 'openshift_prometheus_proxy_image'),
-    ('openshift_prometheus_altermanager_image_prefix', 'openshift_prometheus_alertmanager_image'),
-    # A typo was introduced at some point, need to warn for this older version.
-    ('openshift_prometheus_altermanager_image_prefix', 'openshift_prometheus_alertmanager_image'),
-    ('openshift_prometheus_alertmanager_image_version', 'openshift_prometheus_alertmanager_image'),
-    ('openshift_prometheus_alertbuffer_image_prefix', 'openshift_prometheus_alertbuffer_image'),
-    ('openshift_prometheus_alertbuffer_image_version', 'openshift_prometheus_alertbuffer_image'),
-    ('openshift_prometheus_node_exporter_image_prefix', 'openshift_prometheus_node_exporter_image'),
-    ('openshift_prometheus_node_exporter_image_version', 'openshift_prometheus_node_exporter_image'),
-    ('openshift_descheduler_image_prefix', 'openshift_descheduler_image'),
-    ('openshift_descheduler_image_version', 'openshift_descheduler_image'),
-    ('openshift_docker_gc_version', 'openshift_docker_gc_image'),
-    ('openshift_web_console_prefix', 'openshift_web_console_image'),
-    ('openshift_web_console_version', 'openshift_web_console_image'),
-    ('openshift_web_console_image_name', 'openshift_web_console_image'),
-    ('openshift_storage_glusterfs_version', 'openshift_storage_glusterfs_image'),
-    ('openshift_storage_glusterfs_block_version', 'openshift_storage_glusterfs_block_image'),
-    ('openshift_storage_glusterfs_s3_version', 'openshift_storage_glusterfs_s3_image'),
-    ('openshift_storage_glusterfs_heketi_version', 'openshift_storage_glusterfs_heketi_image'),
-    ('openshift_storage_glusterfs_registry_version', 'openshift_storage_glusterfs_registry_image'),
-    ('openshift_storage_glusterfs_registry_block_version', 'openshift_storage_glusterfs_registry_block_image'),
-    ('openshift_storage_glusterfs_registry_s3_version', 'openshift_storage_glusterfs_registry_s3_image'),
-    ('openshift_storage_glusterfs_registry_heketi_version', 'openshift_storage_glusterfs_registry_heketi_image'),
+    # Leaving example as this code might be used again in future.
+    # ('old_var', 'new_var')
 )
 
 # JSON_FORMAT_VARIABLES does not intende to cover all json variables, but
@@ -133,18 +88,6 @@ JSON_FORMAT_VARIABLES = (
     'openshift_master_open_ports',
 )
 
-# TODO(michaelgugino): Remove in 3.11
-CHANGED_IMAGE_VARS = (
-    'openshift_storage_glusterfs_image',
-    'openshift_storage_glusterfs_block_image',
-    'openshift_storage_glusterfs_s3_image',
-    'openshift_storage_glusterfs_heketi_image',
-    'openshift_storage_glusterfs_registry_image',
-    'openshift_storage_glusterfs_registry_block_image',
-    'openshift_storage_glusterfs_registry_s3_image',
-    'openshift_storage_glusterfs_registry_heketi_image',
-)
-
 
 def to_bool(var_to_check):
     """Determine a boolean value given the multiple
@@ -426,26 +369,6 @@ class ActionModule(ActionBase):
                             'existing master configs, and remove the {} key'
                             'before proceeding.'.format(old_key, old_key))
 
-    def check_contains_version(self, hostvars, host):
-        """Fails if variable has old format not containing image version"""
-        found_incorrect = []
-        for img_var in CHANGED_IMAGE_VARS:
-            img_string = self.template_var(hostvars, host, img_var)
-            if not img_string:
-                return None
-            # split the image string by '/' to account for something like docker://
-            img_string_parts = img_string.split('/')
-            if ':' not in img_string_parts[-1]:
-                found_incorrect.append((img_var, img_string))
-
-        if found_incorrect:
-            msg = ("Found image variables without version.  Please ensure any image"
-                   "defined contains ':someversion'; ")
-            for item in found_incorrect:
-                msg += "{} was: {}; ".format(item[0], item[1])
-            raise errors.AnsibleModuleError(msg)
-        return None
-
     def validate_json_format_vars(self, hostvars, host):
         """Fails if invalid json format are found"""
         found_invalid_json = []
@@ -481,7 +404,6 @@ class ActionModule(ActionBase):
         self.check_unsupported_nfs_configs(hostvars, host)
         self.check_htpasswd_provider(hostvars, host)
         check_for_removed_vars(hostvars, host)
-        self.check_contains_version(hostvars, host)
         self.validate_json_format_vars(hostvars, host)
 
     def run(self, tmp=None, task_vars=None):

roles/lib_utils/filter_plugins/oo_filters.py

@@ -9,7 +9,6 @@ import json
 import os
 import pdb
 import random
-import re
 
 from base64 import b64encode
 from collections import Mapping
@@ -428,33 +427,6 @@ def lib_utils_to_padded_yaml(data, level=0, indent=2, **kw):
         raise errors.AnsibleFilterError('Failed to convert: %s' % my_e)
 
 
-def lib_utils_oo_pods_match_component(pods, deployment_type, component):
-    """ Filters a list of Pods and returns the ones matching the deployment_type and component
-    """
-    # TODO: michaelgugino -- need to find a better method than matching on image
-    # this will break with some of the recent oreg_url changes.
-    if not isinstance(pods, list):
-        raise errors.AnsibleFilterError("failed expects to filter on a list")
-    if not isinstance(deployment_type, string_types):
-        raise errors.AnsibleFilterError("failed expects deployment_type to be a string")
-    if not isinstance(component, string_types):
-        raise errors.AnsibleFilterError("failed expects component to be a string")
-
-    image_prefix = 'docker.io/openshift/origin-'
-    if deployment_type == 'openshift-enterprise':
-        image_prefix = 'registry.access.redhat.com/openshift3/ose-'
-
-    matching_pods = []
-    image_regex = r'.*' + image_prefix + component + r'.*'
-    for pod in pods:
-        for container in pod['spec']['containers']:
-            if re.search(image_regex, container['image']):
-                matching_pods.append(pod)
-                break  # stop here, don't add a pod more than once
-
-    return matching_pods
-
-
 def lib_utils_oo_image_tag_to_rpm_version(version, include_dash=False):
     """ Convert an image tag string to an RPM version if necessary
         Empty strings and strings that are already in rpm version format
@@ -528,26 +500,6 @@ def lib_utils_oo_loadbalancer_backends(
     return loadbalancer_backends
 
 
-def lib_utils_oo_chomp_commit_offset(version):
-    """Chomp any "+git.foo" commit offset string from the given `version`
-    and return the modified version string.
-
-Ex:
-- chomp_commit_offset(None)                 => None
-- chomp_commit_offset(1337)                 => "1337"
-- chomp_commit_offset("v3.4.0.15+git.derp") => "v3.4.0.15"
-- chomp_commit_offset("v3.4.0.15")          => "v3.4.0.15"
-- chomp_commit_offset("v1.3.0+52492b4")     => "v1.3.0"
-    """
-    if version is None:
-        return version
-    else:
-        # Stringify, just in case it's a Number type. Split by '+' and
-        # return the first split. No concerns about strings without a
-        # '+', .split() returns an array of the original string.
-        return str(version).split('+')[0]
-
-
 def lib_utils_oo_random_word(length, source='abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'):
     """Generates a random string of given length from a set of alphanumeric characters.
        The default source uses [a-z][A-Z][0-9]
@@ -558,21 +510,6 @@ def lib_utils_oo_random_word(length, source='abcdefghijklmnopqrstuvwxyzABCDEFGHI
     return ''.join(random.choice(source) for i in range(length))
 
 
-def lib_utils_oo_contains_rule(source, apiGroups, resources, verbs):
-    '''Return true if the specified rule is contained within the provided source'''
-
-    rules = source['rules']
-
-    if rules:
-        for rule in rules:
-            if set(rule['apiGroups']) == set(apiGroups):
-                if set(rule['resources']) == set(resources):
-                    if set(rule['verbs']) == set(verbs):
-                        return True
-
-    return False
-
-
 def lib_utils_oo_selector_to_string_list(user_dict):
     """Convert a dict of selectors to a key=value list of strings
 
@@ -740,7 +677,6 @@ class FilterModule(object):
         return {
             "lib_utils_oo_select_keys": lib_utils_oo_select_keys,
             "lib_utils_oo_select_keys_from_list": lib_utils_oo_select_keys_from_list,
-            "lib_utils_oo_chomp_commit_offset": lib_utils_oo_chomp_commit_offset,
             "lib_utils_oo_collect": lib_utils_oo_collect,
             "lib_utils_oo_pdb": lib_utils_oo_pdb,
             "lib_utils_oo_prepend_strings_in_list": lib_utils_oo_prepend_strings_in_list,
@@ -751,14 +687,12 @@ class FilterModule(object):
             "lib_utils_oo_parse_named_certificates": lib_utils_oo_parse_named_certificates,
             "lib_utils_oo_parse_certificate_san": lib_utils_oo_parse_certificate_san,
             "lib_utils_oo_generate_secret": lib_utils_oo_generate_secret,
-            "lib_utils_oo_pods_match_component": lib_utils_oo_pods_match_component,
             "lib_utils_oo_image_tag_to_rpm_version": lib_utils_oo_image_tag_to_rpm_version,
             "lib_utils_oo_hostname_from_url": lib_utils_oo_hostname_from_url,
             "lib_utils_oo_loadbalancer_frontends": lib_utils_oo_loadbalancer_frontends,
             "lib_utils_oo_loadbalancer_backends": lib_utils_oo_loadbalancer_backends,
             "lib_utils_to_padded_yaml": lib_utils_to_padded_yaml,
             "lib_utils_oo_random_word": lib_utils_oo_random_word,
-            "lib_utils_oo_contains_rule": lib_utils_oo_contains_rule,
             "lib_utils_oo_selector_to_string_list": lib_utils_oo_selector_to_string_list,
             "lib_utils_oo_filter_sa_secrets": lib_utils_oo_filter_sa_secrets,
             "lib_utils_oo_l_of_d_to_csv": lib_utils_oo_l_of_d_to_csv,

roles/lib_utils/meta/main.yml

@@ -1,6 +1,6 @@
 ---
 galaxy_info:
-  author: TODO
+  author: various
   description: OpenShift Repositories
   company: Red Hat, Inc.
   license: Apache License, Version 2.0

roles/openshift_cli/tasks/main.yml

@@ -30,14 +30,3 @@
   when: not openshift_is_atomic | bool
   register: result
   until: result is succeeded
-
-# TODO(michaelgugino) Remove in 3.11.
-- name: Ensure binaries from containerized deployments are cleaned up.
-  file:
-    path: "{{ item }}"
-    state: absent
-  with_items:
-  - /usr/local/bin/oc
-  - /usr/local/bin/openshift
-  - /usr/local/bin/kubectl
-  when: not openshift_is_atomic

roles/openshift_control_plane/defaults/main.yml

@@ -143,11 +143,6 @@ openshift_master_embedded_dns: True
 openshift_master_embedded_kube: True
 openshift_master_pod_eviction_timeout: ""
 
-# TODO(michaelgugino): Remove in 3.11
-l_new_config_clusterNetworks:
-- cidr: "{{ openshift_cluster_network_cidr }}"
-  hostSubnetLength: "{{ openshift_host_subnet_length }}"
-
 # added so that it can lay down the static pod definitions in a configurable place
 openshift_control_plane_static_pod_location: /etc/origin/node/pods/
 openshift_control_plane_apply_cluster_signing_config: True

roles/openshift_control_plane/tasks/migrate_idproviders.yml

@@ -1,18 +0,0 @@
----
-# This file is to update htpass_provider filename path as it now must be
-# hard-coded to /etc/origin/master/htpasswd
-
-# Recreate the htpass file in the new location.
-- import_tasks: htpass_provider.yml
-
-# lib_utils_mutate_htpass_provider is a customer filter in
-# roles/lib_utils/filter_plugins/oo_filters.py
-- name: modify master id providers
-  yedit:
-    src: /etc/origin/master/master-config.yaml
-    edits:
-    - key: oauthConfig.identityProviders
-      value: "{{ openshift_master_existing_idproviders | lib_utils_mutate_htpass_provider }}"
-  when:
-  - openshift_master_existing_idproviders is defined
-  - openshift_master_manage_htpasswd

roles/openshift_control_plane/tasks/upgrade.yml

@@ -8,9 +8,6 @@
   - "{{ openshift_service_type }}-master-controllers"
   failed_when: false
 
-# Only needed for 3.10, remove in 3.11.
-- import_tasks: migrate_idproviders.yml
-
 - import_tasks: static_shim.yml
 
 - import_tasks: upgrade/upgrade_scheduler.yml
@@ -55,46 +52,6 @@
     key: "imagePolicyConfig.internalRegistryHostname"
     value: "docker-registry.default.svc:5000"
 
-# TODO(michaelgugino): Remove in 3.11
-- name: Add new network config section to master conf
-  yedit:
-    src: "{{ openshift.common.config_base }}/master/master-config.yaml"
-    key: networkConfig.clusterNetworks
-    value: "{{ l_new_config_clusterNetworks }}"
-  # l_existing_config_master_config is set via playbooks/init/basic_facts.yml
-  when: l_existing_config_master_config.networkConfig.clusterNetworks is not defined
-
-# TODO(michaelgugino): Remove in 3.11
-- name: Remove old network config section in master conf
-  yedit:
-    src: /etc/origin/master/master-config.yaml
-    key: "{{ item }}"
-    state: absent
-  with_items:
-  - networkConfig.clusterNetworkCIDR
-  - networkConfig.hostSubnetLength
-
-# TODO(michaelgugino): Remove in 3.11
-- name: Check for old master env file
-  stat:
-    path: "/etc/sysconfig/{{ openshift_service_type }}-master-api"
-  register: old_env
-
-# TODO(michaelgugino): Remove in 3.11
-- name: Migrate old master env file to new location
-  master_env_config_migrate:
-    src: "/etc/sysconfig/{{ openshift_service_type }}-master-api"
-    dest: "{{ openshift.common.config_base }}/master/master.env"
-  when: old_env.stat.exists
-
-# TODO(michaelgugino): Remove in 3.11
-- name: mv old master env file to .backup
-  command: "mv {{ l_old_env_file }} {{ l_old_env_file_backup }}"
-  vars:
-    l_old_env_file: "/etc/sysconfig/{{ openshift_service_type }}-master-api"
-    l_old_env_file_backup: "/etc/sysconfig/{{ openshift_service_type }}-master-api.backup"
-  when: old_env.stat.exists
-
 - name: Update oreg value
   yedit:
     src: "{{ openshift.common.config_base }}/master/master-config.yaml"
@@ -127,19 +84,5 @@
     value: "{{ runtime_config.result | join(',') | regex_replace('(?:,)*apis/settings\\.k8s\\.io/v1alpha1=true','') }}"
   when: runtime_config.result
 
-- name: Remove old service information
-  file:
-    path: "{{ item }}"
-    state: absent
-  with_items:
-  - /etc/systemd/system/atomic-openshift-master-api.service
-  - /etc/systemd/system/atomic-openshift-master-controllers.service
-  - /etc/systemd/system/origin-master-api.service
-  - /etc/systemd/system/origin-master-controllers.service
-  - /usr/lib/systemd/system/atomic-openshift-master-api.service
-  - /usr/lib/systemd/system/atomic-openshift-master-controllers.service
-  - /usr/lib/systemd/system/origin-master-api.service
-  - /usr/lib/systemd/system/origin-master-controllers.service
-
 - name: reload systemd units
   command: systemctl daemon-reload