ranger-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Madhan Neethiraj <mad...@apache.org>
Subject Re: Review Request 66085: RANGER-2019: Handle upgrade scenario to rename the old ATLAS service def and use the new service def
Date Sun, 18 Mar 2018 02:02:07 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66085/#review199382
-----------------------------------------------------------




security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 37 (patched)
<https://reviews.apache.org/r/66085/#comment279706>

    Consider using already existing EmbeddedServiceDefsUtil.EMBEDDED_SERVICEDEF_ATLAS_NAME,
instead of defining new constant SERVICEDBSTORE_SERVICEDEFBYNAME_ATLAS_NAME.



security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 90 (patched)
<https://reviews.apache.org/r/66085/#comment279701>

    Why is it critical to bail out when "serviceId != 11"? If the intention is to avoid rename
on multiple runs of this patch, then I would suggest to check for resource-names and access-types
to determine if the service-def is older version or not. If the service-def has all resources
and accessTypes listed below, then it is an older service-def and should be renamed:
    
     - resources: entity, type, operation, taxonomy, term
     - accessTypes: read, create, update, delete, all



security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 91 (patched)
<https://reviews.apache.org/r/66085/#comment279702>

    add a info level log here. 
    
      LOG.info(EMBEDDED_SERVICEDEF_ATLAS_NAME + ": service-def not found. No patching is needed");



security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 95 (patched)
<https://reviews.apache.org/r/66085/#comment279707>

    Consider renaming service-def first. The suffix used to rename the service-def should
be used to rename the services as well.
    
    String          serviceDefName = EMBEDDED_SERVICEDEF_ATLAS_NAME;
    XXServiceDefDao serviceDefDao  = daoMgr.getXXServiceDef();
    XXServiceDef    serviceDef     = serviceDefDao.findByName(serviceDefName);
    
    if (serviceDef == null) {
      LOG.info(serviceDefName + ": service-def not found. No patching is needed");
      
      return;
    }
    
    String suffix = null;
    
    for (int i = 1; true; i++) {
      suffix = ".v" + i;
    
      if (serviceDefDao.findByName(serviceDefName + suffix) == null) {
        break;
      }
    }
    
    String serviceDefNewName = serviceDefName + suffix;
    
    LOG.info("Renaming service-def " + serviceDefName + " as " + serviceDefNewName);
    
    serviceDef.setName(serviceDefNewName);
    
    serviceDefDao.update(serviceDef);
    
    LOG.info("Renamed service-def " + serviceDefName + " as " + serviceDefNewName);
    
    XXServiceDao    serviceDao = daoMgr.getXXService();
    List<XXService> services   = serviceDao.findByServiceDefId(serviceDef.getId());
    
    if (CollectionUtils.isNotEmpty(services)) {
      for (XXService service : services) {
        String serviceName    = service.getName();
        String serviceNewName = serviceName + suffix;
    
        LOG.info("Renaming service " + serviceName + " as " + serviceNewName);
        
        if (serviceDao.findByName(serviceNewName) != null) {
          LOG.warn("Another service named " + serviceNewName + " already exists. Not renaming
" + serviceName);
          
          continue;
        }
    
        service.setName(serviceNewName);
        
        serviceDao.update(service);
    
        LOG.info("Renamed service " + serviceName + " as " + serviceNewName);
      }
    }



security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 103 (patched)
<https://reviews.apache.org/r/66085/#comment279703>

    When would this be false? Should that be considered as a patch failure (instead of ignoring
it)? Atleast a warning should be logged.



security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 104 (patched)
<https://reviews.apache.org/r/66085/#comment279704>

    "Renamed service " + service.getName() + " to " updateServiceName



security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
Lines 111 (patched)
<https://reviews.apache.org/r/66085/#comment279705>

    When would this be false? I think failing to rename should be considered as a patch failure.


- Madhan Neethiraj


On March 17, 2018, 9:33 p.m., Pradeep Agrawal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66085/
> -----------------------------------------------------------
> 
> (Updated March 17, 2018, 9:33 p.m.)
> 
> 
> Review request for ranger, bhavik patel, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj,
Mehul Parikh, Ramesh Mani, Sailaja Polavarapu, and Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2019
>     https://issues.apache.org/jira/browse/RANGER-2019
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> **Problem Statement:** Based on this RANGER-2004 commit, Atlas service def is updated.
During the upgrade of existing env, old Atlas service def should be renamed so that new service
def can be used.
> 
> **Proposed Solution:** Proposed Java patch can address the atlas service def rename fix.
During migration to latest Ranger, this Java patch should do the following:
> 1. Check for service-def named 'atlas'. If it is not found - nothing more to do.
> 2. Rename all service instances of type 'atlas', with addition of suffix "-v1". Existing
service named 'cl1_atlas' will become 'cl1_atlas-v1'. If 'cl1_atlas-v1' already exists, append
an count like 'cl1_atlas-v1.1', 'cl1_atlas-v1.2'
> 3. After all service instances are renamed, rename service-def as 'atlas-v1'.
> 
> **Note:** In Upgrade case, as the old atlas service def id can't be used to create the
new atlas service def so I am proposing change to update the atlas servicedef id from 11 to
15.
> 
> 
> Diffs
> -----
> 
>   agents-common/src/main/resources/service-defs/ranger-servicedef-atlas.json 2891129

>   security-admin/src/main/java/org/apache/ranger/patch/PatchForAtlasServiceDefUpdate_J10013.java
PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66085/diff/1/
> 
> 
> Testing
> -------
> 
> Tested for fresh and upgrade case.
> 
> 
> Thanks,
> 
> Pradeep Agrawal
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message